Closed
Bug 1223690
Opened 7 years ago
Closed 7 years ago
Remove implicit Rect conversions
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla45
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: n.nethercote, Assigned: n.nethercote)
Details
Attachments
(1 file)
18.67 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
The implicit constructor in gfxRect has some subtle and surprising effects. Let's remove it.
![]() |
Assignee | |
Comment 1•7 years ago
|
||
gfxRect can be implicitly constructed from IntRect, which hides a number of implicit conversion points, makes Moz2Dification harder, and has some surprising effects. This patch removes the implicit constructor and replaces it with an explicit conversion function: gfxRect ThebesRect(const IntRect&) This is the obvious outcome of removing the constructor. But there is also a second, less obvious outcome: currently we do a number of IntRect-to-Rect conversions using ToRect(), which (surprisingly) works because it turns into an implicit IntRect-to-gfxRect conversion (via the implicit constructor) combined with an explicit gfxRect-to-Rect conversion (via ToRect()). I.e. we do two conversions, going from a Moz2D type to a Thebes type and back to a Moz2D type! So this patch also changes these conversion. It moves this existing function: Rect ToRect(const IntRect&) from gfx2DGlue.h -- where it doesn't really belong because it doesn't involve any Thebes types -- to gfx/2d/Rect.h, templatifying and renaming it as IntRectToRect() in the process. The rest of the patch deals with fall-out from these changes. The call sites change as follows: - IntRect-to-gfxRect conversions: - old: implicit - new: ThebesRect() - IntRect-to-Rect conversions: - old: ToRect() - new: IntRectToRect()
Attachment #8685868 -
Flags: review?(jmuizelaar)
![]() |
Assignee | |
Comment 2•7 years ago
|
||
Comment on attachment 8685868 [details] [diff] [review] Remove implicit Rect conversions Review of attachment 8685868 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/thebes/gfxBlur.cpp @@ -954,5 @@ > // Dirty rect and skip rect are null for the min inset shadow. > // When rendering inset box shadows, we respect the spread radius by changing > // the shape of the unblurred shadow, and can pass a spread radius of zero here. > IntSize zeroSpread(0, 0); > - gfxContext* minGfxContext = Init(ThebesRect(ToRect(outerRect)), This line is worth highlighting in particular. outerRect is an IntRect, so this line is doing the following conversions: IntRect -(implicit)-> gfxRect -(explicit)-> Rect -(explicit)-> gfxRect The new code just does: IntRect -(explicit)-> gfxRect
Comment 3•7 years ago
|
||
Comment on attachment 8685868 [details] [diff] [review] Remove implicit Rect conversions Review of attachment 8685868 [details] [diff] [review]: ----------------------------------------------------------------- I think I put the implicit conversion, but I'm happy to have it go.
Attachment #8685868 -
Flags: review?(jmuizelaar) → review+
![]() |
Assignee | |
Comment 4•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed48eac74e39a37bf9b1fc21e1ae16699965b19f Bug 1223690 - Remove implicit Rect conversions. r=jrmuizel.
Comment 5•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ed48eac74e39
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in
before you can comment on or make changes to this bug.
Description
•