Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: BenWa, Assigned: BenWa)

Tracking

39 Branch
mozilla42
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 attachment)

Right now the minimap breaks down when the page has multiple APZ. Facebook with the chat open is a common example but they are not very rare. The minimap will all render at the top left of the container layer which will cause a giant mess.

Next I want to stop rendering tiny minimaps for trivial frames.
Whiteboard: [gfx-noted]
Bug 1189611 - Improve the APZ minimap position and ignore tivial APZ. r=kats
Attachment #8641495 - Flags: review?(bugmail.mozilla)
Comment on attachment 8641495 [details]
MozReview Request: Bug 1189611 - Improve the APZ minimap position and ignore trivial APZ. r=kats

Bug 1189611 - Improve the APZ minimap position and ignore trivial APZ. r=kats
Attachment #8641495 - Attachment description: MozReview Request: Bug 1189611 - Improve the APZ minimap position and ignore tivial APZ. r=kats → MozReview Request: Bug 1189611 - Improve the APZ minimap position and ignore trivial APZ. r=kats
Assignee: nobody → bgirard
Comment on attachment 8641495 [details]
MozReview Request: Bug 1189611 - Improve the APZ minimap position and ignore trivial APZ. r=kats

https://reviewboard.mozilla.org/r/14559/#review13141

::: gfx/layers/composite/ContainerLayerComposite.cpp:366
(Diff revision 2)
>    LayerRect viewRect = ParentLayerRect(scrollOffset, fm.GetCompositionBounds().Size()) / LayerToParentLayerScale(1);

s/fm.GetCompositionBounds()/compositionBounds/ on this line

::: gfx/layers/composite/ContainerLayerComposite.cpp:370
(Diff revision 2)
> +  if (viewRect.width < 64 && viewRect.height < 64) {

Can we rearrange the code a bit so we don't uselessly compute scrollRect and dp if we're going to return early? (i.e. move those variable declarations down to after the early-return).
Attachment #8641495 - Flags: review?(bugmail.mozilla) → review+
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #3)
> ::: gfx/layers/composite/ContainerLayerComposite.cpp:370
> (Diff revision 2)
> > +  if (viewRect.width < 64 && viewRect.height < 64) {
> 
> Can we rearrange the code a bit so we don't uselessly compute scrollRect and
> dp if we're going to return early? (i.e. move those variable declarations
> down to after the early-return).

IMO this just makes the code a bit harder to read. We're taking about a non performance sensitive area (trivial computation) and is only hit under debug for rare case. I'd prefer to keep the code clear here if you don't mind. I'd rather not make this change.
Comment on attachment 8641495 [details]
MozReview Request: Bug 1189611 - Improve the APZ minimap position and ignore trivial APZ. r=kats

Bug 1189611 - Improve the APZ minimap position and ignore trivial APZ. r=kats
Keywords: checkin-needed
Blocks: 1189924
https://hg.mozilla.org/mozilla-central/rev/48510c8af0be
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.