[webkit-reviews] review denied: [Bug 202739] [GTK][WPE] Renderization of Conic gradients : [Attachment 385117] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Dec 9 12:58:12 PST 2019


Carlos Alberto Lopez Perez <clopez at igalia.com> has denied Diego Pino
<dpino at igalia.com>'s request for review:
Bug 202739: [GTK][WPE] Renderization of Conic gradients
https://bugs.webkit.org/show_bug.cgi?id=202739

Attachment 385117: Patch

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




--- Comment #16 from Carlos Alberto Lopez Perez <clopez at igalia.com> ---
Comment on attachment 385117
  --> https://bugs.webkit.org/attachment.cgi?id=385117
Patch

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

> Source/WebCore/ChangeLog:6
> +	   Reviewed by Carlos Alberto Lopez Perez.

Please remove this and leave the "reviewed by nobody oops", I still haven't
reviewed this with an r+ :)

> Source/WebCore/platform/graphics/cairo/GradientCairo.cpp:59
> +static void addColorStopRGBA(cairo_pattern_t* gradient, Gradient::ColorStop
thisColor, float globalAlpha);
> +
> +#if PLATFORM(GTK) || PLATFORM(WPE)
> +
> +typedef struct point_t {
> +    double x, y;
> +} point_t;
> +
> +static void addConicSector(cairo_pattern_t* gradient, float cx, float cy,
float r, float angleRadians,
> +    Gradient::ColorStop firstColor, Gradient::ColorStop lastColor, float
globalAlpha);
> +static cairo_pattern_t* createConic(float xo, float yo, float r, float
angleRadians,
> +    Gradient::ColorStopVector stops, float globalAlpha);
> +static Gradient::ColorStop interpolateColorStop(Gradient::ColorStop
firstColor, Gradient::ColorStop lastColor);
> +static void setCornerColorRGBA(cairo_pattern_t* gradient, int id,
Gradient::ColorStop thisColor, float globalAlpha);
> +
> +#endif
> +

This should be moved to Gradient.h

> Source/WebCore/platform/graphics/cairo/GradientCairo.cpp:76
> +#if PLATFORM(GTK) || PLATFORM(WPE)
> +	   [&] (const ConicData& data)	-> cairo_pattern_t* {
> +	       return createConic(data.point0.x(), data.point0.y(),
data.startRadius, data.angleRadians, stops(), globalAlpha);
> +#else
>	   [&] (const ConicData&)  -> cairo_pattern_t* {
>	       // FIXME: implement conic gradient rendering.
>	       return nullptr;
> +#endif

Is something on this implementation that is GTK/WPE specific and won't work on
other platforms using Cairo? Otherwise, maybe its a good idea to not ifdef the
implementation?

> Source/WebCore/platform/graphics/cairo/GradientCairo.cpp:117
> +#if PLATFORM(GTK) || PLATFORM(WPE)

Ditto

> Source/WebCore/platform/graphics/cairo/GradientCairo.cpp:120
> +static cairo_pattern_t* createConic(float xo, float yo, float r, float
angleRadians,
> +    Gradient::ColorStopVector stops, float globalAlpha)

This can be written in one line. In webkit style code is ok to write the
function header in one very long line instead of breaking it in several

> Source/WebCore/platform/graphics/cairo/GradientCairo.cpp:156
> +static void addConicSector(cairo_pattern_t *gradient, float cx, float cy,
float r, float angleRadians,
> +    Gradient::ColorStop from, Gradient::ColorStop to, float globalAlpha)

Ditto.


More information about the webkit-reviews mailing list