Fix ascent and descent calculation in panorama

[lexi-lambda]

Fixes #63.

[rfindler]

This one seems, a priori, worrying from a backwards compatibility perspective. But since it only affects the descent in the result of panorama then, at least in the way I tend to use that function (I feel like I don't use baseline appending or superimposition on panorama'd picts), maybe this is possible.

(I'll try running some old talks and see if things look broken and report back. I tried running some old talks and I don't anything that looks broken.)

[lexi-lambda]

Thanks for checking! I agree that this is a much more significant change than my other recent PRs.

My rationale for just making this change was that this is very clearly a bug: the existing behavior is both strange and somewhat unpredictable. Intentionally depending on the current behavior seems very difficult, since I couldn’t even understand what it was doing until I carefully read the implementation. On the other hand, it wouldn’t surprise me if someone somewhere was depending on it unintentionally.

But what’s the alternative, really? A new #:correct-baseline? keyword option? A parameter? pict-2? I’m not sure what I’d do instead.

[rfindler]

I understand your rationale. I don't think the "it is a bug" rationale is sufficient. "It is a bug and no one seems to depend on it" is more to my taste for a rationale that passes muster. My best hope is that that's what happens and maybe others will contribute. Maybe you can too if you like, by finding code and trying things out (not that you have to, of course).

As for alternatives, I would point to the #:draw-border? argument to filled-rectangle which, it seems to me, is exactly analogous. It papers over a bug and I have memorized that argument because I use it routinely and I have never been in a situation where I wanted the default value (only in situations where the default value didn't break things enough that I cared).

[lexi-lambda]

As for alternatives, I would point to the #:draw-border? argument to filled-rectangle?

I agree that the default there is rarely what I want, but at least in that case there are some legitimate uses. I am sympathetic to your broader point, though.

I will admit it is unlikely that I will take the time to actively audit other people’s code to see how this affects things, but I also don’t feel any particular urgency to get this fixed. I can just run off my fork for now, so I’m happy to wait and let other people weigh in. If this fix is ultimately deemed too incompatible, so be it.

[rfindler]

My experiments are positive which, in addition to pointing out that there are uses of panorama where this change doesn't matter (just like with filled-rectangle), is good, but I would feel better if there were more.

[mflatt]

I have few uses of panorama, but the change is ok for those as far as I see.

[lexi-lambda]

I realized today that the ascent calculation is actually also incorrect, it just shows up in different situations. So I’ve now pushed a change that fixes that, too.

[rfindler]

Should there be two test cases?

[rfindler]

It seems safe to merge this. Any objections?

[soegaard]

Ping.