Closed Bug 1681610 Opened 5 years ago Closed 5 years ago

Bullet points have a hole at the centre

Categories

(Core :: Graphics, defect)

Product:
Core
Component:
Graphics
Version:
Firefox 83
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
86 Branch
Tracking Flags:
Tracking Status
firefox-esr78 --- unaffected
firefox84 --- wontfix
firefox85 --- fixed
firefox86 --- fixed

People

(Reporter: ygoe, Assigned: jimb)

References

(Regression)

Details

(Keywords: regression)

Attachments

(4 files)

firefox-bullet-points-hole.png
20.46 KB, image/png
Details
Level 1 and 2 lists on GitHub markdown
5.64 KB, image/png
Details
Bug 1681610: Use thicker borders when faking rounded rectangles. r?gw
48 bytes, text/x-phabricator-request
jcristau
: approval-mozilla-beta+
Details | Review
Another screenshot of rendering diversity - source: https://unosquare.github.io/raspberryio/
19.01 KB, image/png
Details

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:83.0) Gecko/20100101 Firefox/83.0

Steps to reproduce:

Open any web page that uses <ul><li> and look at the bullet points. For a while now they're not filled but have a tiny hole at the centre. Some are filled but they look slightly squashed vertically (ellipse, not circle), maybe by 1px. Could have to do with a non-integral line height (careless use of fractional values and the em unit) and rounding glitches in layout. A common problem.

My monitor has a text scaling of 100%. I could also test at 200% on another device.

Examples:

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Component: Untriaged → Graphics
Product: Firefox → Core

Those list icons look funny. The first level looks like a vinyl record and the second level seems to indicate a little inverted plus symbol in a filled circle. Something has changed there recently, and it probably went wrong.

Mozregression points to bug 1669394. Jim, would you mind taking a look?

Severity: -- → S3
Flags: needinfo?(jimb)
Keywords: regression
Regressed by: 1669394
Has Regression Range: --- → yes

Yes, that change could totally be responsible for this. Taking.

Flags: needinfo?(jimb)
Assignee: nobody → jimb

I can reproduce this.

(Kind of upset I didn't notice it when I made the change. I wonder why the reftests didn't catch this.)

On Linux, this is all it takes to reproduce for me:

<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8">
    <style>
      body {
          font-family: Nimbus Sans
                       /* -apple-system, BlinkMacSystemFont, Segoe UI, Helvetica, Arial, sans-serif, Apple Color Emoji, Segoe UI Emoji */
          ;
      }
    </style>
  </head>
  <body>
    <p>Here's a list:
    <ul>
      <li>certain things</li>
      <li>whereas other things</li>
    </ul>
  </body>
</html>

Zooming in and out helps. You'll get holes at some zoom levels and not others.

DisplayListBuilder::PushRoundedRect, used for unordered list bullets and the
like, draws rounded rectangles as ordinary rectangles with rounded borders that
have a radius of half the rectangle. Unfortunately, this leads to tiny white
dots at the center of the bullet under some magnifications. I haven't diagnosed
why - perhaps there's something somewhere that snaps borders outward.

Increasing the thickness of the borders to (an impossible) 60% of the width of
the rectangle makes the problem go away. If this kludge doesn't work, we could
add a rectangle covering the center.

Hm. Isn't something wrong with the implementation if a shape as complex as as rounded rectangle with a certain border width and then even an additional rectangle renders faster than a plain (filled) circle? From what I've learned in the Computer Graphics course at university, I'd believe that the latter is much simpler. Are you overcomplicating or am I oversimplifying things?

It's funny to see the full diversity of renderings of such a simple HTML element. Yellow highlighting added by me. (Again GitHub, I seem to be there a lot and they use bullet lists the most.) All screenshots taken at the default 100% scaling for page and screen. This is what everybody (with a low-DPI screen) sees when they don't mess with their view settings.

Attachment #9195408 - Attachment description: Another screenshot of rendering diversity → Another screenshot of rendering diversity - source: https://unosquare.github.io/raspberryio/
Pushed by jblandy@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/35e51eb9d864 Use thicker borders when faking rounded rectangles. r=gw

https://hg.mozilla.org/mozilla-central/rev/35e51eb9d864

Status: UNCONFIRMED → RESOLVED
Closed: 5 years ago
status-firefox86: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch

The patch landed in nightly and beta is affected.
:jimb, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(jimb)

This is a minor cosmetic issue, but the patch is low-risk, so I think it would be good to uplift to beta.

Flags: needinfo?(jimb)

(In reply to Yves Goergen from comment #9)

Hm. Isn't something wrong with the implementation if a shape as complex as as rounded rectangle with a certain border width and then even an additional rectangle renders faster than a plain (filled) circle? From what I've learned in the Computer Graphics course at university, I'd believe that the latter is much simpler. Are you overcomplicating or am I oversimplifying things?

I think you're oversimplifying things.

One thing that happens in implementing the web is that common cases get optimized. If we drew something as common as HTML element borders using more general primitives like filled paths, it would require more complexity to achieve a given level of performance than simply treating element borders as a primitive in their own right, and writing custom code to handle them. Every browser implementation makes concessions of generality to accommodate observed use patterns.

In this particular case, element borders appear as a primitive operation in WebRender display lists (see DisplayItem::Border), and these borders are cached, so once one bullet is rendered, subsequent bullets with equivalent parameters are just composited out of a cache texture. There is no primitive operation for circles: we want the set of display list items we support to be as small as possible, to reduce complexity.

[edit: it turns out that drawing an additional rectangle would indeed perform poorly, for a number of reasons]

Comment on attachment 9195358 [details]
Bug 1681610: Use thicker borders when faking rounded rectangles. r?gw

Beta/Release Uplift Approval Request

  • User impact if declined: Unordered list item bullets will have tiny white pixels in the center, at some magnification levels.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The fix just changes a division by two into a multiplication by 0.6 in a size computation.
  • String changes made/needed: none
Attachment #9195358 - Flags: approval-mozilla-beta?

Oh, does this also cover the funny inverted plus icons in the bullet points of the second level? (See provided screenshots.) Or did you only remove the white hole in the points of the first level?

(In reply to Yves Goergen from comment #18)

Oh, does this also cover the funny inverted plus icons in the bullet points of the second level? (See provided screenshots.) Or did you only remove the white hole in the points of the first level?

In CSS terms, the first-level bullets are disc bullets, and the second-level bullets are circle bullets. I think 'circle' bullets have always looked like that at lower resolutions. Bug 1669394 should have had no effect on their appearance, in any case.

While I have no screenshots at hand, I'm close to certain that circle bullets have once looked like circles at normal (traditional) resolution. Something has changed here, too, I first didn't even recognise that as a circle bullet but thought it was a custom image.

Comment on attachment 9195358 [details]
Bug 1681610: Use thicker borders when faking rounded rectangles. r?gw

approved for 85.0b9

Attachment #9195358 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

(In reply to Yves Goergen from comment #20)

While I have no screenshots at hand, I'm close to certain that circle bullets have once looked like circles at normal (traditional) resolution. Something has changed here, too, I first didn't even recognise that as a circle bullet but thought it was a custom image.

I really don't think circle bullets have changed, but if you want to verify, the mozregression tool makes it really easy and convenient to try older versions of Firefox: https://mozilla.github.io/mozregression/

Looks like the situation was handled only very partly. In Firefox 85, two issues remain and a new one might exist now.

  1. The second-level bullet points still look like an inverted plus icon in a filled circle, not a circle outline.
  2. With fractional line heights, the first-level bullet points still have inconsistent height. This was better before that weird optimisation was added.
  3. (NEW) The first-level bullet points look slightly bigger now.

I'll have to spend more time and test this with older versions of Firefox, so I can provide screenshots of the issue in its entirety. It's definitely bigger than was was resolved here by now.

I'm absolutely sure they've changed. I wouldn't have noticed! I'm not sure you guys have a sharp vision and a classic monitor that runs at 100% text scaling. But if you did, you'd see it. It's still very much annoying to have a) inconsistently sized bullet points and b) little plus icons instead of circle outlines. You can see how the outline bullet point looked in the past in the screenshots already provided, just watch out for those that have the wrong size. They're much closer to the previous (and correct) state than the others. But they're the wrong size, they're not even circles but ellipses. Please, if you're interested in providing decent graphics quality, then fix this properly not in a haste like this.

This whole border drawing thing smells like premature optimisation to me. You should know the old wisdom: Make it correct. Make is fast. In that order. Currently, the first requirement is not met, making the second useless. I can easily produce crap that nobody wants at arbitrary speed.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: