Closed Bug 2055145 Opened 3 days ago Closed 1 day ago

Some letters have their bottom part cropped in the address bar

Categories

(Core :: Graphics: WebRender, defect)

Firefox 154
Desktop
All
defect

Tracking

()

RESOLVED FIXED
154 Branch
Tracking Status
firefox-esr140 --- unaffected
firefox152 --- unaffected
firefox153 --- unaffected
firefox154 --- fixed

People

(Reporter: cbaica, Assigned: gw)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files)

Affected versions

  • Fx 154.0a1

Affected platforms

  • Windows 11
  • Ubuntu 24

Steps to reproduce

  1. Launch Firefox and open a new tab.
  2. Type 'g' and 'y' in the address bar.
  3. Observe the bottom part of the letters.

Expected result

  • Letters are fully displayed.

Actual result

  • Bottom part of the letters are cropped.

Regression range

Additional notes

  • Issue is 100% reproducible on an Ubuntu machine with 1920x1200 resolution and a Windows machine with 1920x1080(recommended) display resolution and 125% scale (recommended).
  • Issue is reproducible in the search bar as well.
  • Issue is reproducible with browser.nova.enabled set to true as well.
  • I've managed to reproduce the issue on a desktop machine by setting the resolution to 1920x1080 and 125% scale, though these were not the recommended settings. Issue is not reproducible on the same machine with 1920x1080 and 100% scale or 2560x1440 and 100% scale.
See Also: → 2053709

Thanks, this is most likely the same as bug 2050821. Not 100% clear but I don't think it's helpful to keep multiple bugs open that are so similar. Ardelean found the same regressor and much of the same notes. Comment 0 is so similar to the latest comments in that bug -- were you aware of it when you filed this one? Do you view this one as being different from that one?

Status: NEW → RESOLVED
Closed: 3 days ago
Duplicate of bug: 2050821
Resolution: --- → DUPLICATE
See Also: 2050821

Hey Drew,
Yes, I was aware of the bug. I talked it over with Oana Ardelean and I got to submit the issue as she was caught in different tasks for today. We got the impression from comment 8 that you wanted a new issue submitted for the regression range she discovered.

Flags: needinfo?(adw)

Oh sorry, I see. Thanks for filing a new one. It seemed like a near copy of bug 2050821, and I was expecting a Graphics bug that set the regressor to bug 2050692. OK, let's reopen it, move it to Graphics, and set the regressor.

Status: RESOLVED → REOPENED
Component: Address Bar → Graphics: WebRender
No longer duplicate of bug: 2050821
Flags: needinfo?(adw)
Keywords: regression
Product: Firefox → Core
Regressed by: 2050692
Resolution: DUPLICATE → ---
See Also: → 2050821

Set release status flags based on info from the regressing bug 2050692

:gw, since you are the author of the regressor, bug 2050692, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Flags: needinfo?(mozilla)

I haven't been able to reproduce this, with the exact reported config (Ubuntu machine with 1920x1200 resolution). What Ubuntu version is that running - 26.04? Is there anything else changed in terms of system settings etc that may affect whether I can repro this?

Flags: needinfo?(mozilla) → needinfo?(cbaica)

I noticed above you mentioned Ubuntu 24 - I've also tried reproducing a build with current m-c code on Ubuntu 24.04 LTS at 100% DPI on a 1920x1200 screen - no luck so far, so there must be some other detail that affects whether this reproduces.

This also happens on Windows fwiw, see bug 2050821 comment 7 for info. There may be multiple bugs here, it's not clear, because bug 2050821 was filed before bug 2050692 landed, but QA found a regression range that points to bug 2050692, and that bug does seem possibly related. I'm wondering if the original problem is a front-end bug, and then bug 2050692 made it worse or at least interfered with reproducing it.

If you're able to provide any insight from a graphics perspective, it would be appreciated.

(In reply to Drew Willcoxon :adw from comment #7)

This also happens on Windows fwiw, see bug 2050821 comment 7 for info. There may be multiple bugs here, it's not clear, because bug 2050821 was filed before bug 2050692 landed, but QA found a regression range that points to bug 2050692, and that bug does seem possibly related. I'm wondering if the original problem is a front-end bug, and then bug 2050692 made it worse or at least interfered with reproducing it.

If you're able to provide any insight from a graphics perspective, it would be appreciated.

Yep. It's hard to say without being able to repro locally (I have tried on windows too), but I'm doing some investigation of the exact font sizes, clip rects, subpixel bounding boxes etc that frontend/gecko is providing to webrender.

The patch that was marked as a regressor should result in the text rendering more accurately than it was previously. Definitely could be a bug, but my current theory based on the numbers I'm seeing is that the font size + subpixel AA is either right on or just over the clip-rect edge that is supplied to webrender.

Should have some detailed numbers soon.

More specifically - bug https://bugzilla.mozilla.org/show_bug.cgi?id=2050692 definitely does change the location of some subpixel text rendering very slightly, but the intent (sans bugs) is that it's more accurate than the previous rendering approach in terms of exact subpixel positioning.

OK, I think I have fairly clear picture what's going on here, and there's two things happening:

(a) There is a bug in https://bugzilla.mozilla.org/show_bug.cgi?id=2050692 - that bug attempts to fix the way fractional clipping occurs on fractional text runs.

However, the way text runs draw now is different depending on the subpixel positioning direction. In the (default) horizontal left-to-right text, we don't snap the horizontal glyph position at all, depending on subpixel positioning. For the non-subpixel-positioned dir, we do snap the glyph positions, by snapping the vertical baseline to a device pixel to avoid blurriness.

The patch in question unconditionally avoids pixel snapping the clip rect(s) that apply to the text run - this is correct for the horizontal direction, but invalid for the vertical direction. If a text run gets snapped vertically by +/- 0.5 device pixels, we need to account for that in the way the clip-rects are snapped.

I should have a patch up later today which switches the clip-snapping policy for text runs to be per-axis, rather than unconditional, which I believe will fix most of this.

(b) The font size in the front end for the URL bar is such that the height of the clip rect is right on the boundary of where issues can be expected.

It's a tight enough clip rect that any movement of the text run by +/- 0.5 device pixels can be enough to have it clipped. I haven't delved deeply in to the front end CSS, but it looks like the font-size is scaled by a 1.05 factor on Linux, I think? This, combined with a fractional font size to begin with (I see a font size passed to webrender of 15.4063 px) are going to expose any floating point accuracy issues in hardware or the pipeline.

I think it's also quite likely that if a user selects a font that has a slightly larger descender than we expect that we'll see clipping anyway?

This is probably worth an audit of the front end code, and possibly loosening slightly the clip-rect bounds (which I assume are derived from the height of the url bar, but maybe something else?).

Flags: needinfo?(adw)

A device-space text run's clip was left exact/unsnapped (bug 2050692). Because
the glyph is snapped to the device grid on its non-sub-pixel axis, an exact
fractional clip edge there shaves a whole glyph row - e.g. cropped g/y
descenders in the address bar. Round the clip out on the non-sub-pixel axis
while keeping the sub-pixel axis exact, so glyph rows are never shaved and the
sub-pixel axis stays identical to the bug 2050692 behavior.

The rounding is scoped to text-run clips alone. A prim's device-grid snap
policy is a SnapPolicy { rect, clip }: the bounding rect rounds independently of
the clips, and the clip rounding is an exhaustive ClipSnap - Nearest for
snapping prims, Exact for surfaces and any other device-space non-text prim
(their clips stay exact, as before), and Text(..) for a text run (round out on
the non-sub-pixel axis). set_active_clips consumes the ClipSnap directly; the
tile-cache caller derives its own (never a text run, so Nearest or Exact).

Adds a wrench reftest (bug2055145) and a space.rs unit test for the per-axis
snap policy.

Assignee: nobody → mozilla

The attached patch has an in-progress try run [1] for all platforms that should help make this better (addressing (a) from comment 10). Since this is very specific to user configuration, if anyone was able to reproduce locally previously, pulling the builds from the linked try run and confirming here if this resolves or improves the bug would be very helpful.

[1] https://treeherder.mozilla.org/jobs?repo=try&revision=6527f742a701b566299a4935687f0ac9d78cb952

(In reply to Glenn Watson [:gw] from comment #5)

I haven't been able to reproduce this, with the exact reported config (Ubuntu machine with 1920x1200 resolution). What Ubuntu version is that running - 26.04? Is there anything else changed in terms of system settings etc that may affect whether I can repro this?

Unfortunately I don't have any additional info. The ubuntu version I'm running is 24.04 and all the display settings are displayed on the end of the recording. Not sure if screen size matters, but I'm using a small laptop with 15" display in the screen recording.

Flags: needinfo?(cbaica)

Thanks for the very quick investigation and fix! The text in the urlbar is in an <html:input>, and presumably the clip rect is the bounds of the input? When I was testing on Windows earlier, I found that the bottommost glyph row (to borrow your term) of the letter g touches the bottom of the input. There's a screenshot here: https://phabricator.services.mozilla.com/D312197#10829901

So if I understand you correctly, it's not hard to imagine it getting clipped. That screenshot is in patch that increases the height of the input by a pretty good amount. With these two patches, we can be pretty confident that this is fixed I think?

Flags: needinfo?(adw)

Yes, those screenshots you have look like a good improvement to me.

The more I think about the attached patch above, the less certain I am it's the way to go. This is one of those cases where there's not necessarily a clearly correct behavior here - there are pros and cons to how it currently works compared to what changes with the attached patch. It's one of those leaky abstractions where having to align to pixels for crisp text gets in the way of the pure float math.

I will ponder a little more, but I'm actually tempted to hold off on landing this patch if you can resolve it reasonably in the frontend, and see if we get any other bug reports on real world web content that lead me to a clearer picture of which behavior is most "correct" when we encounter a fractional text baseline very close to a fractional clip rect.

Thoughts?

Flags: needinfo?(adw)

Green try run with the patch if we do decide to land it (some fuzz on a few mac wpt tests was needed):

https://treeherder.mozilla.org/jobs?repo=try&revision=7309484ed9e1fc1f2e8a617029d47ec7fae3a85e

Here's a screenshot of the urlbar's <html:input> at its intrinsic height [1]. The bottom of the "g" still touches the bottom of the input. Was it like that before your patch? If it was, does your patch make it more likely that the bottom row of pixels will get clipped? These are questions you might be better positioned to answer than me. If the answers are Yes, then if it were me I'd strongly consider reverting the part of your patch that affected vertical positioning, especially since as you say there's not really a right answer.

[1] i.e. without CSS rules that stretch or shrink it. At least, I'm pretty sure anyway -- I disabled all the relevant rules on the input and its DOM ancestors, but it's so complex that it's hard to say if I got them all without spending more time on it. If you do run some more tests, please double check me with an isolated <html:input>.

Flags: needinfo?(adw)

That's helpful information, thanks. I used claude to do a detailed sweep across many fonts, dpi ratios, at each of the 3 code states (before either patch, after 1st patch, and after the attached patch) using your intrinsic height idea. It does find cases where there is some clipping after the 1st patch, but no clipping after the 2nd patch. So I'm going to land the patch in this bug and we'll see how it goes in terms of regressing anything.

Report pasted below so that we know the exact cases which show clipping if we need to revisit this.


Method

- Three builds from one tree: pre-regressor (before bug 2050692), regressor (exact clip), fix (D312414, round-out on the non-sub-pixel axis).
- Instrumented WebRender (WR_TEXTCLIP_DEBUG) to log, per device-space text run, the rasterized glyph ink bbox vs the run's clip, and the shave outcome under both the exact clip and round-out. The regressor build thus reports current behavior and the fix's prediction from identical glyph geometry.
- Repro page: an <input> at intrinsic height (no explicit height; only font-size/font-family varied), value "gjpqy", swept over font-size; DPR via layout.css.devPixelsPerPx (non-headless required). The input uses padding:0 (worst-case tight clip; a real urlbar has padding, hence more margin).
- Numeric shave validated against the foreground eyeball in both directions, so the numbers are a trusted proxy.

Results

Descender-to-clip margin (positive = safe), Noto Sans, by DPR:

┌──────┬────────────────────┬──────────────┬──────────────────┐
│ DPR  │ min margin (exact) │ exact shaves │ round-out shaves │
├──────┼────────────────────┼──────────────┼──────────────────┤
│ 1.0  │ +0.000             │ 0            │ 0                │
├──────┼────────────────────┼──────────────┼──────────────────┤
│ 1.25 │ -0.500             │ many         │ 0                │
├──────┼────────────────────┼──────────────┼──────────────────┤
│ 1.33 │ -0.333             │ some         │ 0                │
├──────┼────────────────────┼──────────────┼──────────────────┤
│ 1.5  │ -0.500             │ some         │ 0                │
├──────┼────────────────────┼──────────────┼──────────────────┤
│ 1.75 │ +0.529             │ 0            │ 0                │
├──────┼────────────────────┼──────────────┼──────────────────┤
│ 2.0  │ +0.500             │ 0            │ 0                │
└──────┴────────────────────┴──────────────┴──────────────────┘

Font-agnostic at DPR 1.25 and 1.5: exact clip shaves ~0.5px at many sizes; round-out shaves 0 in every case. Tested Noto Sans, DejaVu Sans, Liberation Sans, Cantarell (urlbar font here), Noto Serif, Noto Sans Mono. Cantarell worst: 29 shaving sizes at DPR 1.25.

Visual confirmation (DPR 1.25, Noto Sans): sizes 10.8-12.0px (device ~13.5-15px) clearly clipped on the regressor, correct on the fix; 10.5-10.75px correct on both - matching the numeric prediction exactly, including the beat.

Conclusion

- Intrinsic height alone does not always prevent the crop: at DPR 1.25 the exact clip visibly shaves descenders across all common fonts. It is a fractional beat of effective-font-size x DPR, hence config-sensitive.
- D312414 removes every shave, for every font and DPR tested, and never under-clips.
- Not tested: whether round-out's outward expansion (up to ~1px vertically) can reveal content meant to be clipped (e.g. overflow:hidden text) - the one theoretical downside, left as a follow-up.
Pushed by gwatson@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/ab90048d5830 https://hg.mozilla.org/integration/autoland/rev/b6943dbb3d8b Round out device-space text-run clips on the non-sub-pixel axis. r=gfx-reviewers,lsalzman
Status: REOPENED → RESOLVED
Closed: 3 days ago1 day ago
Resolution: --- → FIXED
Target Milestone: --- → 154 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: