Unintentionally creating layers for non-overlay scrollbars and resizers

RESOLVED FIXED in mozilla32

Status

()

Core
Layout
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mstange, Assigned: mstange)

Tracking

Trunk
mozilla32
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
Created attachment 8431675 [details] [diff] [review]
only force layerizization of actual overlay scrollbars

I forgot that on Mac, non-overlay scrollbars are position:relative, and so they get built in the second AppendScrollPartsTo pass. So my patch in bug 1016535 gave non-overlay scrollbars on Mac their own layers, too, even though they could have merged with the background layer just fine.

This patch alone will not fix bug 1017256 because in the meantime bug 1009679 has also landed which comes with the same additional layerization that made tscrollx slower.
Attachment #8431675 - Flags: review?(roc)
Comment on attachment 8431675 [details] [diff] [review]
only force layerizization of actual overlay scrollbars

>@@ -2674,19 +2678,18 @@ ScrollFrameHelper::BuildDisplayList(nsDi
>     // In case we are not using displayport or the nsDisplayScrollLayers are
>     // flattened during visibility computation, we still need to export the
>     // metadata about this scroll box to the compositor process.
>     nsDisplayScrollInfoLayer* layerItem = new (aBuilder) nsDisplayScrollInfoLayer(
>       aBuilder, mScrolledFrame, mOuter);
>     scrolledContent.BorderBackground()->AppendNewToBottom(layerItem);
>   }
>   // Now display overlay scrollbars and the resizer, if we have one.
>-  // Always create layers for these, so that we don't create a giant layer
>-  // covering the whole scrollport if both scrollbars are visible.
>-  AppendScrollPartsTo(aBuilder, aDirtyRect, scrolledContent, true, true);
>+  AppendScrollPartsTo(aBuilder, aDirtyRect, scrolledContent,
>+                      createLayersForScrollbars, true);
>   scrolledContent.MoveTo(aLists);
> }

Don't you also need this change at the top of the function for the other set of AppendScrollPartsTo calls in the early exit for ignoring the scroll frame?
(Assignee)

Comment 2

4 years ago
You're right.
(Assignee)

Comment 3

4 years ago
Created attachment 8431948 [details] [diff] [review]
only force layerizization of actual overlay scrollbars
Attachment #8431675 - Attachment is obsolete: true
Attachment #8431675 - Flags: review?(roc)
Attachment #8431948 - Flags: review?(roc)
https://hg.mozilla.org/mozilla-central/rev/5bcb4980eb17
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.