For the layout test images/color-profile-svg.html, the expected and actual output of the layout test are attached.
The color profile of the image used by the SVG is ignored.
Interestingly, this bug only occurs when the output color space is sRGB (or may be has an sRGB transfer function). When the output color space different (gamma 1.8), the output is correct.
I see similar failures in
images/color-profile-svg.html
images/color-profile-svg-fill-text.html
images/color-profile-group.html
images/color-profile-background-image-space.html
virtual/gpu-rasterization/images/color-profile-svg.html
In SkImage::makeColorSpace, we have the early-out if (!this->colorSpace() && target->isSRGB()) ... and *this* doesn't have a color space yet. But it should.
My guess is that something about the load-an-image-through-an-SVG path doesn't require that the size and color space be read as early as they need to be.
The problem comes up in SkPictureShader::refBitmapShader. When we hit this call, dstColorSpace is nullptr (cause we're in legacy mode), but SkPictureShader::fColorSpace is set to the color space we want to end up in.
We create tileImage by SkImage::MakeFromGenerator(SkPictureImageGenerator::Make(...)).
tileImage's image info will never have a color space (it's a bunch of draw commands, after all), and we're in legacy mode (so we shouldn't be using dstColorSpace, since that's for SkSurfaces, which we don't want here).
We call tileImage->makeColorSpace(fColorSpace).
Only if fColorSpace is non-SRGB will be do color conversion here. The stack for the conversion will be
SkImage_Lazy::directGeneratePixels(...
SkImage_Lazy::onMakeColorSpace(...
SkImage::makeColorSpace(...
SkPictureShader::refBitmapShader(...
-----(paths diverge here)------
SkPictureShader::onMakeContext(...
SkShaderBase::makeContext(...
SkBlitter::Choose(...
SkAutoBlitterChoose::SkAutoBlitterChoose(...
SkAutoBlitterChoose::SkAutoBlitterChoose(...
SkDraw::drawRect(...
SkDraw::drawRect(...
SkBitmapDevice::drawRect(...
SkCanvas::onDrawRect(...
SkCanvas::drawRect(...
SkColorSpaceXformCanvas::onDrawRect(...
SkCanvas::drawRect(...
If fColorSpace is sRGB, then tileImage will do no conversion here, and we'll be reading the image with the stack
SkCanvas::drawImageRect(...
SkCanvas::legacy_drawImageRect(...
void SkRecords::Draw::draw<SkRecords::DrawImageRect>(...
void SkRecords::Draw::operator(...
decltype(...
decltype(...
SkRecordDraw(...
SkBigPicture::playback(...
SkCanvas::drawPicture(...
SkCanvas::drawPicture(...
** SkPictureImageGenerator::onGetPixels(...
SkImageGenerator::getPixels(...
SkImageGenerator::getPixels(...
generate_pixels(...
SkImage_Lazy::lockAsBitmap(...
SkImage_Lazy::getROPixels(...
SkBitmapProvider::asBitmap(...
SkDefaultBitmapControllerState::SkDefaultBitmapControllerState(...
SkDefaultBitmapControllerState::SkDefaultBitmapControllerState(...
SkDefaultBitmapControllerState* SkInPlaceNewCheck<...
SkDefaultBitmapController::onRequestBitmap(...
SkBitmapController::requestBitmap(...
SkBitmapProcInfo::init(...
SkBitmapProcState::setup(...
SkBitmapProcLegacyShader::MakeContext(...
-----------
SkImageShader::onMakeContext(...
SkShaderBase::makeContext(...
SkPictureShader::PictureShaderContext::PictureShaderContext(...
SkPictureShader::PictureShaderContext::PictureShaderContext(...
SkPictureShader::PictureShaderContext* SkArenaAlloc::make<...
SkPictureShader::onMakeContext(...
SkShaderBase::makeContext(...
SkBlitter::Choose(...
SkAutoBlitterChoose::SkAutoBlitterChoose(...
SkAutoBlitterChoose::SkAutoBlitterChoose(...
SkDraw::drawRect(...
SkDraw::drawRect(...
SkBitmapDevice::drawRect(...
SkCanvas::onDrawRect(...
SkCanvas::drawRect(...
SkColorSpaceXformCanvas::onDrawRect(...
SkCanvas::drawRect(...
If we go drill into the call at SkPictureImageGenerator::onGetPixels at the (**), we see that this has a SkCreateColorSpaceXformCanvas path that we don't hit.
I'm not too clear as to in which circumstances we want to be setting a color space on these objects for legacy mode or for non-legacy mode.
So, questions that I'm thinking about are:
1. What should a SkImage::MakeFromGenerator return as its colorSpace()?
Should it lazily do onMakeColorSpace?
Should it just say "sRGB" all the time?
How do we specify that without accidentally hitting non-legacy mode?
2. Should SkPictureImageGenerator::onGetPixels be hitting the color transform path here?
IIUC, SkImage::makeColorSpace() takes an early exit because it thinks that converting nullptr->sRGB is a no-op.
Since the SkImage is picture backed, we actually do need to make sure that we render using a SkColorSpaceXformCanvas with the dst set to sRGB.
I'll fix this.
Thanks, this fix makes sense. I was a bit iffy on the idea of getting rid of the "un-tagged source" optimization, but the SkImage sub-classes will just return themselves from the makeColorSpace call.
Actual result color-profile-svg-actual.png in the OP looks odd.
If I open its test case in Safari, on a screen with whacked color profile (to match was the layout test does), here is the result (attached).
This may or may not work for you, depending on what bugs.chromium.org does with image color profiles.
Comment 1 by ccameron@chromium.org
, Jun 3 2017