Closed
Bug 1084093
Opened 10 years ago
Closed 10 years ago
Undo the breakage to gfx.color_management.mode=1 (eCMSMode_All) caused by the Moz2D porting work
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: jwatt, Assigned: jwatt)
Details
Attachments
(3 files)
1.39 KB,
patch
|
bas.schouten
:
review+
jwatt
:
checkin+
|
Details | Diff | Splinter Review |
7.65 KB,
patch
|
bas.schouten
:
review+
jwatt
:
checkin+
|
Details | Diff | Splinter Review |
19.80 KB,
patch
|
bas.schouten
:
review+
jwatt
:
checkin+
|
Details | Diff | Splinter Review |
I just noticed that I and my reviewers failed to notice that when I've been converting code that used to call nsRenderingContext::SetColor to Moz2D, we've been losing the gfxPlatform::MabyeTransformColor() call in gfxContext::SetColor().
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8506469 -
Flags: review?(bas)
Assignee | ||
Comment 2•10 years ago
|
||
We're going to be making a lot of calls to what is currently gfxPlatform::MaybeTransformColor, so I really want a shorter name since call points invariably want that to keep the code tidy. Hence the rename to ToDeviceColor in the mozilla::gfx namespace. ("device" seeming appropriate given gfxContext's SetDeviceColor, and it's the best name I could think of.)
Attachment #8506476 -
Flags: review?(bas)
Assignee | ||
Comment 3•10 years ago
|
||
It seems like as a general rule we should use ToDeviceColor whenever we create a ColorPattern.
Attachment #8506503 -
Flags: review?(bas)
Assignee | ||
Comment 4•10 years ago
|
||
Rather than having lots of Mozilla call points call ToDeviceColor() (error prone) it would seem better long term to have qcms integrated into Moz2D. The number of places where PatternType::COLOR is processed in Moz2D code is much smaller than the number of places where we are (and are going to) call ToDeviceColor() in the gecko code. For now though, this unbreaks things and gives us a way to proceed with the Moz2D porting work.
Updated•10 years ago
|
Attachment #8506469 -
Flags: review?(bas) → review+
Comment 5•10 years ago
|
||
Comment on attachment 8506476 [details] [diff] [review] part 2 - Convert gfxPlatform::TransformPixel to Moz2D, and move gfxPlatform::MaybeTransformColor to ToDeviceColor in gfxUtils.h Review of attachment 8506476 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/thebes/gfxUtils.cpp @@ +1366,5 @@ > + return deviceColor; > + } > + } > + deviceColor = aColor; > + return deviceColor; nit: Any reason not just to return aColor and put deviceColor in the scope of the if?
Attachment #8506476 -
Flags: review?(bas) → review+
Updated•10 years ago
|
Attachment #8506503 -
Flags: review?(bas) → review+
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8506469 [details] [diff] [review] part 1 - Add some helpers to Moz2D Color to convert to/from packed ARGB https://hg.mozilla.org/integration/mozilla-inbound/rev/cb91f5ec6b44
Attachment #8506469 -
Flags: checkin+
Assignee | ||
Comment 7•10 years ago
|
||
Comment on attachment 8506476 [details] [diff] [review] part 2 - Convert gfxPlatform::TransformPixel to Moz2D, and move gfxPlatform::MaybeTransformColor to ToDeviceColor in gfxUtils.h https://hg.mozilla.org/integration/mozilla-inbound/rev/6d94aa5503e5
Attachment #8506476 -
Flags: checkin+
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8506503 [details] [diff] [review] part 3 - Use the new ToDeviceColor() function to undo the breakage to gfx.color_management.mode=1 (eCMSMode_All) caused by the Moz2D porting work https://hg.mozilla.org/integration/mozilla-inbound/rev/8714fdc213ca
Attachment #8506503 -
Flags: checkin+
Assignee | ||
Comment 9•10 years ago
|
||
Follow-up for missing include: https://hg.mozilla.org/integration/mozilla-inbound/rev/0b5504a244dd
Comment 10•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/cb91f5ec6b44 https://hg.mozilla.org/mozilla-central/rev/6d94aa5503e5 https://hg.mozilla.org/mozilla-central/rev/8714fdc213ca https://hg.mozilla.org/mozilla-central/rev/0b5504a244dd
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•