Closed Bug 1084093 Opened 5 years ago Closed 5 years ago

Undo the breakage to gfx.color_management.mode=1 (eCMSMode_All) caused by the Moz2D porting work

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: jwatt, Assigned: jwatt)

Details

Attachments

(3 files)

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().
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)
It seems like as a general rule we should use ToDeviceColor whenever we create a ColorPattern.
Attachment #8506503 - Flags: review?(bas)
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.
Attachment #8506469 - Flags: review?(bas) → review+
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+
Attachment #8506503 - Flags: review?(bas) → review+
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+
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+
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+
You need to log in before you can comment on or make changes to this bug.