Status

()

defect
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: roc, Assigned: roc)

Tracking

Trunk
x86
macOS
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

We call aBuilder->ToReferenceFrame(mFrame) a lot, like on every GetBounds(). This traverses frames all the way up to the display root, every time. Instead, we should compute aBuilder->ToReferenceFrame(mFrame) once, on display item construction, and use the cached result afterwards.
Mostly just adding nsDisplayListBuilder* parameters to constructors, so that the nsDisplayItem constructor can initialize mToReferenceFrame.
Attachment #462689 - Flags: review?(tnikkel)
Comment on attachment 462689 [details] [diff] [review]
Part 1: Add nsDisplayItem::ToReferenceFrame

     NS_ENSURE_SUCCESS(rv, rv);
     rv = aLists.Outlines()->AppendNewToTop(new (aBuilder)
-        nsDisplayXULDebug(this));
+        nsDisplayXULDebug(Builder this));
     NS_ENSURE_SUCCESS(rv, rv);

You successfully compiled this after fixing this part, right?
Attachment #462689 - Flags: review?(tnikkel) → review+
Comment on attachment 462690 [details] [diff] [review]
Part 2: Convert code to use nsDisplayItem::ToReferenceFrame

Is this going to make it harder if we keep around display lists?
Attachment #462690 - Flags: review?(tnikkel) → review+
(In reply to comment #3)
> You successfully compiled this after fixing this part, right?

Yes, but that code is in DEBUG_LAYOUT which is not usually defined. I fixed the error though.

(In reply to comment #4)
> Is this going to make it harder if we keep around display lists?

No. If we ever do keep around display lists, we'll still be rebuilding them from scratch every time we need a new one.

In any case, I doubt we will keep around display lists per se. The approach I'm pursuing for invalidation is to record salient information from old display lists in FrameLayerBuilder::DisplayItemData.
Whiteboard: [needs landing]
Whiteboard: [needs landing]
Whiteboard: [needs approval]
Attachment #462689 - Flags: approval2.0? → approval2.0+
Attachment #462690 - Flags: approval2.0? → approval2.0+
Whiteboard: [needs approval] → [needs landing]
I landed this, but backed it out in case it was responsible for test failures. But it wasn't guilty (reproduced test failures on try server without this patch). However, it might be responsible for Txul regression on Mac 10.5.8 so we shouldn't reland until that's checked (hard to believe though).
Whiteboard: [needs landing]
Would be nice to get this landed, might help with 130078 perf.
You need to log in before you can comment on or make changes to this bug.