erkyrath / lectrote

The IF interpreter in an Electron shell
Other
248 stars 28 forks source link

TADS 2 status window goes blank at non-100% browser zoom #133

Closed erkyrath closed 2 years ago

erkyrath commented 3 years ago

Noted on Worlds Apart.

https://intfiction.org/t/lectrote-1-4-0/50036

curiousdannii commented 3 years ago

Could the measurements be breaking perhaps?

erkyrath commented 3 years ago

Possibly related: the width measurement of the input line also gets messed up at non-100% zoom.

https://intfiction.org/t/lectrote-1-4-0/50036

curiousdannii commented 2 years ago

Assuming that it's the same as for Parchment, it looks like the top/bottom values that GlkOte calculates are often incorrect when zoomed.

First lets remember that in a modern browser, zooming changes the devicePixelRatio, as if you were using a hiDPI screen. This means that calculations using pixels should still work and keep the same ratios. Because in browsers CSS pixels never actually refer to true device pixels any more.

As you zoom in or out, the padding on the status window remains at 6px top and bottom. The internal dimensions are then what's left over from the bottom height. For default styles it wants 18px. But when you zoom in or out, it could change, to 15.035, 18.008, 9.021, 14.020, 16, etc. The status window text will only be shown when it is exactly 18px. Oddly even when it went to 18.008 it did not.

The fact that the browser is zoomed in shouldn't matter - it should get the total viewport size, calculate the height of the status window (which should be unchanged), and then subtract it to get the bottom value for the window.

dfabulich commented 2 years ago

I spent a few minutes today attempting to debug this in Parchment. In accept_windowset I see that arg.gridheight is 0 when zoomed in and arg.gridheight is 1 when unzoomed, which kinda explains the problem, but arg.gridheight is coming out of a magical WASM buffer that I can't figure out how to trace.

Where is the magic happening here??

curiousdannii commented 2 years ago

Hmm, that suggests the metrics are being recalculated wrongly.

dfabulich commented 2 years ago

Well, measure_window seems to generate basically cromulent answers, but I can’t figure out who’s supposed to use that data to compute gridheight…

dfabulich commented 2 years ago

I think rounding up fixes this in both forks of glkote.

curiousdannii commented 2 years ago

I'm still confused as to how the measurements become different purely by zooming.

dfabulich commented 2 years ago

I'm hazy on the details, but I believe browsers adjust text size when zooming, and that text-size-adjust has something to do with it. https://developer.mozilla.org/en-US/docs/Web/CSS/text-size-adjust

erkyrath commented 2 years ago

Fixed in GlkOte, I hope.

curiousdannii commented 2 years ago

I see that AsyncGlk has the same problem.

So the issue only presents itself with TADS. The solution above is to round the metrics, so I'm wondering if the issue is that Remglk can't handle non-integer metrics, but GlkApi can.

Metrics at 110% zoom:

buffercharheight: 20.99431610107422 buffercharwidth: 8 buffermarginx: 10.474400000000003 buffermarginy: 0 graphicsmarginx: 0 graphicsmarginy: 0 gridcharheight: 17.997159957885742 gridcharwidth: 8.4002125 gridmarginx: 20 gridmarginy: 11.988599999999998 height: 324.545 width: 900

It also has the weird situation that RemGlk is sending a 0 height grid window, when I thought it should not send it in that situation?

Should AsyncGlk also round the metrics, or would it be possible to make RemGlk handle non-integer metrics? These height metrics are almost round numbers, but the character widths are definite fractions, and really should be used rather than rounding up. Well @erkyrath must have realised that, because the character metrics are stored as floats:

struct data_metrics_struct {
    glui32 width, height;
    glui32 outspacingx, outspacingy;
    glui32 inspacingx, inspacingy;
    double gridcharwidth, gridcharheight;
    glui32 gridmarginx, gridmarginy;
    double buffercharwidth, buffercharheight;
    glui32 buffermarginx, buffermarginy;
    glui32 graphicsmarginx, graphicsmarginy;
};

So is the issue that the margins and window sizes are integers? Hmm, but I still can't work out what calculations would lead it to in this situation, with a one line grid window, then set the buffer window height to 298px.

And the metrics for 125% zoom:

buffercharheight: 21.000001907348633 buffercharwidth: 8 buffermarginx: 10.075000000000003 buffermarginy: 0 gridcharheight: 18.000001907348633 gridcharwidth: 8.4 gridmarginx: 20 gridmarginy: 12 height: 285.6 width: 900

I think simply rounding up is not the ideal solution, because then these measurements will be almost 1px too high.

Any thoughts @erkyrath?

erkyrath commented 2 years ago

Now that I look at RemGlk, it should accept non-integer metrics.

metrics->gridcharheight = data_raw_real_value(dat);

Can you tell why it isn't?

curiousdannii commented 2 years ago

But only for the charheight/width. So it's probably flooring the height and marginy metrics. And the final calculated window sizes must be ints too

typedef struct grect_struct {
    int left, top;
    int right, bottom;
} grect_t;
curiousdannii commented 2 years ago

It wasn't too hard to get RemGlk to use doubles throughout, which I think is probably the better solution than rounding up.

See the PR above.

However even if that PR is accepted, it will still need to be tested in Lectrote as I didn't test it against GlkOte, just AsyncGlk. And then https://github.com/erkyrath/glkote/commit/d7b5fc79f3a47684b142145b71c96775d78c7e27 should be reverted.

erkyrath commented 2 years ago

Agreed.

erkyrath commented 2 years ago

Should be fixed with the next TADS/etc interpreter updates.