New issue
Advanced search Search tips

Issue 768855 link

Starred by 0 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature

Blocking:
issue 634542



Sign in to add a comment

Add DCHECK for color space in ImageBitmap

Project Member Reported by zakerinasab@chromium.org, Sep 26 2017

Issue description

Add a DCHECK in ImageBitmap to make sure that all image bitmap sources do have a color space when they are passed to createImageBitmap. This ensures that the pipeline is color correct.
 
Trying CHECK at the beginning of ImageBitmap::ApplyColorSpace(). The following image bitmap sources pass the check:

- SVG image, HTMLCanvasElement, ImageData, ImageBitmap, OffscreenCanvas

The following are failing:

- HTMLImageElement (except SVG): failing. Decoders return null color space SkBitmap even when the color behavior is set to TransformToSRGB.
- HTMLVideoElement, Blob

Comment 2 by junov@chromium.org, Sep 27 2017

So we probably have actual color correction bugs here (unrelated to DCHECK) perhaps validate that with images and video that have a color spin profile, and create new tests.
I'm not sure if we have color correction bugs. It seems that we have traces of legacy/null color space in different locations that we need to fix.
After discussion with junov@, it seems that we can add the DCHECK to the
ImageBitmap constructors from ImageElementBase and ImageData. We can't
do this for the constructors from following sources:

HTMLVideoElement, HTMLCanvasElement, OffscreenCanvas, ImageBitmap,
StaticBitmapImage.

The reason is that to keep the alpha disposition in gamma encoded space,
we need to keep the color space of the ImageBufferSurface as nullptr (so
we can wrap the SkiaPaintCanvas in a proper SkColorSpaceXfromCanvas later).

I'll land a CL to add DCHECK to constructors from ImageElementBase and ImageData and keep the bug open for future follow up.


Regarding comment #1, the beginning of ApplyColorSpace() is not a good place to
make sure the pipeline is color correct, as it is not the first place that we
can put the check. Putting the check inside constructors and before consuming/modifying the source seems to be the right place.
Blocking: 634542
Cc: -junov@chromium.org
On a second try, the main issue here is that CanvasResourceProvider still uses SkColorSpaceXformCanvas in legacy mode, hence setting the surface/canvas color space to nullptr when it is asked to set to sRGB. This results in every image bitmap source that is using a CanvasResourceProvider, like HTMLVideoElement, to end up in an Image with no color space information.
If this is ever fixed, remove non-sRGB condition from the color space check in CanvasRenderingContext2DTest::ImageBitmapColorSpaceConversion.
Owner: ----
Status: Available (was: Assigned)

Sign in to add a comment