Closed
Bug 1258481
Opened 8 years ago
Closed 8 years ago
Add a RegionBuilder for accumulating rects
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: BenWa, Unassigned)
References
(Blocks 2 open bugs)
Details
(Keywords: feature, Whiteboard: [gfx-noted])
Attachments
(2 files)
In bug 1248044 we discussed adding a RegionBuilder. Splitting this off here.
Reporter | ||
Comment 1•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/41529/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/41529/
Attachment #8733040 -
Flags: review?(jmuizelaar)
Reporter | ||
Comment 2•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/41523/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/41523/
Attachment #8733041 -
Flags: review?(jmuizelaar)
Comment 3•8 years ago
|
||
Comment on attachment 8733040 [details] MozReview Request: Bug 1258481 - Add a RegionBuilder for accumulating rects. r=jrmuizel https://reviewboard.mozilla.org/r/41529/#review37955
Attachment #8733040 -
Flags: review?(jmuizelaar) → review+
Comment 4•8 years ago
|
||
Comment on attachment 8733041 [details] MozReview Request: Bug 1258481 - Use RegionBuilder for nsRegion IPC. r=jrmuizel https://reviewboard.mozilla.org/r/41523/#review37957 ::: gfx/ipc/GfxMessageUtils.h:27 (Diff revision 1) > -#include "FilterSupport.h" > #include "mozilla/layers/GeckoContentController.h" > +#include "mozilla/layers/LayersTypes.h" > +#include "nsRect.h" > +#include "nsRegion.h" > + I assume all of the include changes make sense.
Attachment #8733041 -
Flags: review?(jmuizelaar) → review+
Reporter | ||
Comment 5•8 years ago
|
||
I just did vim ':sort' on them since they were a mess.
Updated•8 years ago
|
Reporter | ||
Comment 6•8 years ago
|
||
Comment on attachment 8733041 [details] MozReview Request: Bug 1258481 - Use RegionBuilder for nsRegion IPC. r=jrmuizel Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41523/diff/1-2/
Reporter | ||
Comment 7•8 years ago
|
||
Had a simple logic error in the IPC code. ni? for landing after try.
Flags: needinfo?(bgirard)
Reporter | ||
Comment 8•8 years ago
|
||
Comment on attachment 8733041 [details] MozReview Request: Bug 1258481 - Use RegionBuilder for nsRegion IPC. r=jrmuizel Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41523/diff/2-3/
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e60a574fcec https://hg.mozilla.org/integration/mozilla-inbound/rev/8e7a8791854f
Comment 10•8 years ago
|
||
backed out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=24434970&repo=mozilla-inbound
Comment 11•8 years ago
|
||
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/af8f8a484581 https://hg.mozilla.org/integration/mozilla-inbound/rev/52195522dbac
Reporter | ||
Comment 12•8 years ago
|
||
Comment on attachment 8733040 [details] MozReview Request: Bug 1258481 - Add a RegionBuilder for accumulating rects. r=jrmuizel Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41529/diff/1-2/
Reporter | ||
Comment 13•8 years ago
|
||
Comment on attachment 8733041 [details] MozReview Request: Bug 1258481 - Use RegionBuilder for nsRegion IPC. r=jrmuizel Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41523/diff/3-4/
Comment 14•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/89d767660c10 https://hg.mozilla.org/integration/mozilla-inbound/rev/fea97c450559
Reporter | ||
Updated•8 years ago
|
Flags: needinfo?(bgirard)
Comment 15•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/89d767660c10 https://hg.mozilla.org/mozilla-central/rev/fea97c450559
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment 16•8 years ago
|
||
Benoit, is this something manual QA should be looking into for Fx48? If that's the case, could you please expand a bit on what should we focus on while testing this feature?
Flags: qe-verify?
Flags: needinfo?(bgirard)
Reporter | ||
Comment 17•8 years ago
|
||
No need. This is a low level change covered by unit tests.
Flags: needinfo?(bgirard)
Comment 18•8 years ago
|
||
(In reply to Benoit Girard (:BenWa) from comment #17) > No need. This is a low level change covered by unit tests. Thank you for following up on this!
Flags: qe-verify? → qe-verify-
Reporter | ||
Comment 19•8 years ago
|
||
I had a quick look and this way of region building is still O(n^2), more precisely it's at least O(numOfXYBands * numRects).
You need to log in
before you can comment on or make changes to this bug.
Description
•