[webkit-reviews] review denied: [Bug 234118] REGRESSION(r286765): [Cairo][GTK][WPE] Various SVG tests that use filters fail : [Attachment 459747] Patch

[bugzilla-daemon at webkit.org]

Fujii Hironori <Hironori.Fujii at sony.com> has denied Fujii Hironori
<Hironori.Fujii at sony.com>'s request for review:
Bug 234118: REGRESSION(r286765): [Cairo][GTK][WPE] Various SVG tests that use
filters fail
https://bugs.webkit.org/show_bug.cgi?id=234118

Attachment 459747: Patch

https://bugs.webkit.org/attachment.cgi?id=459747&action=review




--- Comment #6 from Fujii Hironori <Hironori.Fujii at sony.com> ---
Comment on attachment 459747
  --> https://bugs.webkit.org/attachment.cgi?id=459747
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=459747&action=review

>> Source/WTF/wtf/PlatformEnable.h:252
>> +#define ENABLE_DESTINATION_COLOR_SPACE_LINEAR_SRGB 0
> 
> That's the right move, one shouldn't assume all graphics backends support
color spaces.
> *sigh* what ever happened to 'https://core.ac.uk/download/pdf/14701464.pdf' ?

Quickly searching through Cairo ML archive,
I found that Adrian Johnson suggested color space support ten years ago, based
on the Andrea Canciani work. But, I found out no progress.

[cairo] Color space API
https://lists.cairographics.org/archives/cairo/2012-July/023353.html

>> Source/WebCore/platform/graphics/cairo/ImageBufferCairoBackend.cpp:65
>> +#if ENABLE(DESTINATION_COLOR_SPACE_LINEAR_SRGB)
> 
> Isn't the intention to assure that the call sites, don't pass LinearSRGB in
the Cairo case?
> 
> One example from an existing call-site:
> 
> #if ENABLE(DESTINATION_COLOR_SPACE_LINEAR_SRGB)
>     auto colorSpace = DestinationColorSpace::LinearSRGB();
> #else
>     auto colorSpace = DestinationColorSpace::SRGB();
> #endif
> 
>     auto sourceGraphic =
context->createScaledImageBuffer(filterData->drawingRegion, filterScale,
colorSpace, filterData->filter->renderingMode());
> 
> ...
> 
> If so, we should assert that	the color spaces are not LinearSRGB fo the
destination if !ENABLE(DESTINTAION_COLOR_SPACE_LINEAR_SRGB).
> At least that's how I understand the current code.

Agreed. Will fix.