Closed
Bug 1361668
Opened 7 years ago
Closed 7 years ago
Correct nsDisplayColumnRules's bound
Categories
(Core :: Graphics: WebRender, enhancement)
Core
Graphics: WebRender
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: ethlin, Assigned: ethlin)
References
Details
Attachments
(1 file, 1 obsolete file)
12.80 KB,
patch
|
mattwoodrow
:
review+
|
Details | Diff | Splinter Review |
Some xul tests[1] will fail after using background color layer. It seems like the visible region of the painted layer is wrong when we create background color layer. [1] layout/reftests/w3c-css/received/css-multicol-1/multicol-rule-large-001.xht
Assignee | ||
Comment 1•7 years ago
|
||
Oh...It's not xul reftest. Looks like the visible region of the painted layer will be wrong after enabling color layer.
Summary: Fix xul test failures after enabling background color layer → Fix visible region problem after enabling background color layer
Assignee | ||
Comment 2•7 years ago
|
||
Looks like the bound[1] of nsDisplayColumnRule is different from it's painting area[2]. I will fix the bound. [1] https://dxr.mozilla.org/mozilla-central/source/layout/generic/nsColumnSetFrame.cpp?q=nsDisplayColumnRule&redirect_type=direct#37 [2] https://dxr.mozilla.org/mozilla-central/source/layout/generic/nsColumnSetFrame.cpp?q=nsDisplayColumnRule&redirect_type=direct#225
Summary: Fix visible region problem after enabling background color layer → Correct nsDisplayColumnRules's bound
Assignee | ||
Comment 3•7 years ago
|
||
Attachment #8864766 -
Flags: review?(matt.woodrow)
Comment 4•7 years ago
|
||
Comment on attachment 8864766 [details] [diff] [review] Fix the bound of nsDisplayColumnRules Review of attachment 8864766 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/nsColumnSetFrame.cpp @@ +158,5 @@ > + return nsRect(linePt, aRuleSize); > +} > + > +nsRect > +nsColumnSetFrame::CalculateBounds(const nsPoint& aPt) This function is almost identical to CreateBorderRenders, except for a few different lines that run per-child. Can you instead make an iterator function that computes the isVertical/isRTL/contentRect etc, and then executes a lambda for each child? That way CalculateBounds could be something like { nsRect combined; ForEachColumn((nsRect lineRect)[combined] { combined.Union(combined, lineRect) }); return combined; } @@ +286,5 @@ > } else { > nscoord edgeOfPrev = prevFrame->GetRect().XMost() + aPt.x; > nscoord edgeOfNext = nextFrame->GetRect().X() + aPt.x; > linePt = nsPoint((edgeOfPrev + edgeOfNext - ruleSize.width) / 2, > contentRect.y); This code is in GetLineRect, so should be removed from here, right?
Assignee | ||
Comment 5•7 years ago
|
||
Okay, I create an iterator function 'ForEachColumn()' for CalculateBounds and CreateBorderRenderers. Not very sure if the naming is proper.
Attachment #8864766 -
Attachment is obsolete: true
Attachment #8864766 -
Flags: review?(matt.woodrow)
Attachment #8865296 -
Flags: review?(matt.woodrow)
Comment 6•7 years ago
|
||
Comment on attachment 8865296 [details] [diff] [review] Fix the bound of nsDisplayColumnRules Review of attachment 8865296 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/nsColumnSetFrame.cpp @@ +190,5 @@ > + } > +} > + > +nsRect > +nsColumnSetFrame::CalculateBounds(const nsPoint& aPt) aOffset? @@ +250,5 @@ > skipSides |= mozilla::eSideBitsRight; > } > > + ForEachColumn([presContext, aCtx, border, aDirtyRect, > + skipSides, this, &aBorderRenderers] I think you could just use [&] to just default to capturing everything needed by reference rather than listing all the variables here.
Attachment #8865296 -
Flags: review?(matt.woodrow) → review+
Assignee | ||
Comment 7•7 years ago
|
||
(In reply to Matt Woodrow (:mattwoodrow) from comment #6) > Comment on attachment 8865296 [details] [diff] [review] > Fix the bound of nsDisplayColumnRules > > Review of attachment 8865296 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: layout/generic/nsColumnSetFrame.cpp > @@ +190,5 @@ > > + } > > +} > > + > > +nsRect > > +nsColumnSetFrame::CalculateBounds(const nsPoint& aPt) > > aOffset? Okay. > > @@ +250,5 @@ > > skipSides |= mozilla::eSideBitsRight; > > } > > > > + ForEachColumn([presContext, aCtx, border, aDirtyRect, > > + skipSides, this, &aBorderRenderers] > > I think you could just use [&] to just default to capturing everything > needed by reference rather than listing all the variables here. Okay, I will use [&].
Pushed by ethlin@mozilla.com: https://hg.mozilla.org/projects/graphics/rev/f9fc89f8b6ff Fix the bound of nsDisplayColumnRules. r=mattwoodrow
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Comment 9•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f9fc89f8b6ff
Updated•7 years ago
|
status-firefox55:
--- → fixed
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•