Split nsLayoutUtils::PaintFrame into two functions, one for build display list, one for paint display list

NEW
Unassigned

Status

()

3 years ago
10 months ago

People

(Reporter: u459114, Unassigned)

Tracking

49 Branch
Points:
---

Firefox Tracking Flags

(firefox49 affected)

Details

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
TODO item of bug 1269971
https://hg.mozilla.org/mozilla-central/rev/f71a8d46fdf2#l1.18
+  // TBD: we actually generate display list of aFrame twice here. It's better
+  // to reuse the same display list and paint that one twice, one for selection
+  // background, one for generating text mask.

To fix it, we can separate nsLayoutUtils::PaintFrame into two functions
1. nsLayoutUtils::BuildDisplayListForFrame()
2. nsLayoutUtils::PaintDisplayList()

With these two functions, we can call nsLayoutUtils::BuildDisplayListForFrame to build a display list once, and paint that display list twice via nsLayoutUtils::PaintDisplayList.
I hadn't seen any of this code before, so I have a few questions:

How does invalidation work for background items that are clipped to text? I can't see any way that the nsDisplayBackgroundImage would get marked as invalid if the text frame children changed.

I guess it might work for most cases since we'll also have the nsDisplayText items in the main display list (assuming we don't optimize them away for being invisible), but we can't guarantee that these actually end up in the same layer as the background (background-attachment:fixed?).

nsLayoutUtils::PaintFrame does BuildDisplayListForStackingContext, but I don't think background-clip:text forces a stacking context. I can't think of anything right now that would be broken by that, but the code certainly doesn't expect it.

I see that we skip building nsDisplayTransform when building display lists for text clipping, but what about all the other container items we build there? SVGEffects and BlendModes could change the rendering of the text, do we want that?
(Reporter)

Comment 2

3 years ago
Created attachment 8752644 [details]
text animation
(Reporter)

Comment 3

3 years ago
Created attachment 8752646 [details]
Blur effect
(Reporter)

Comment 4

3 years ago
(In reply to Matt Woodrow (:mattwoodrow) from comment #1)
> I hadn't seen any of this code before, so I have a few questions:
> 
> How does invalidation work for background items that are clipped to text? I
> can't see any way that the nsDisplayBackgroundImage would get marked as
> invalid if the text frame children changed.
Thanks. It's a bug, see attachment 8752644 [details]. Dirty region is not correct. I will file another issue to fix it.
(Reporter)

Comment 5

3 years ago
(In reply to Matt Woodrow (:mattwoodrow) from comment #1)
> SVGEffects and BlendModes could change the rendering of the text, do we want that?
We still apply all filters and blendmode, but since most of them will not change the shape of text element, except blur effect, you can not see any visual change before and after add a filter or change blend mode.
(Reporter)

Comment 6

3 years ago
(In reply to Matt Woodrow (:mattwoodrow) from comment #1)
> nsLayoutUtils::PaintFrame does BuildDisplayListForStackingContext, but I
> don't think background-clip:text forces a stacking context. I can't think of
> anything right now that would be broken by that, but the code certainly
> doesn't expect it.
I will look into it
(Reporter)

Updated

10 months ago
Assignee: cku → nobody
You need to log in before you can comment on or make changes to this bug.