Bug 227895 – [Cairo] Don't pass the current GraphicsContext state to CairoOperations

• Component: Platform
• Status: NEW

[Fujii Hironori]

[Cairo] Don't pass the current GraphicsContext state to CairoOperations

r279658 (Bug 227721) changed CairoOperations to take a
GraphicsContextCairo as the first argument. No longer need to
pass the current state.

[Fujii Hironori]

Created attachment 433386 [details]
Patch

[Zan Dobersek]

Comment on attachment 433386 [details]
Patch

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

> Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp:672
> -    auto& state = this->state();
> -    append(createCommand<DrawEllipse>(rect, state.fillColor, state.strokeStyle, state.strokeColor, state.strokeThickness));
> +    append(createCommand<DrawEllipse>(rect));

How does the replay context get hold of all the relevant state that's not included in the PaintingOperation structs anymore but is then used in Cairo::drawEllipse() or other similar functions?

[Fujii Hironori]

Comment on attachment 433386 [details]
Patch

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

>> Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp:672
>> +    append(createCommand<DrawEllipse>(rect));
> 
> How does the replay context get hold of all the relevant state that's not included in the PaintingOperation structs anymore but is then used in Cairo::drawEllipse() or other similar functions?

Good point. CairoOperationRecorder::updateState should call GraphicsContextCairo::updateState. Will fix. Thank you.

[Said Abou-Hallawa]

I have a question.

Why can't we merge CairoOperations.cpp in GraphicsContextCairo.cpp? What is the point of having functions like these:

void drawRect(GraphicsContextCairo& platformContext, const FloatRect& rect, ...)
{
    cairo_t* cr = platformContext.cr();
    cairo_save(cr);

    fillRectWithColor(cr, rect, fillColor);

    ...

    cairo_restore(cr);
}

void GraphicsContextCairo::drawRect(const FloatRect& rect, float borderThickness)
{
    ASSERT(!rect.isEmpty());
    auto& state = this->state();
    Cairo::drawRect(*this, rect, borderThickness, state.fillColor, state.strokeStyle, state.strokeColor);
}

Why do not we move the code inside Cairo::drawRect() to GraphicsContextCairo::drawRect especially Cairo::drawRect() takes an argument of type GraphicsContextCairo?

[Fujii Hironori]

(In reply to Said Abou-Hallawa from comment #4)

That's exactly what I want to do.
Bug 227780 – [Cairo] All functions in CairoOperations.cpp should be GraphicsContextCairo's methods

However, I found out that this is not simple as what I thought.
CairoOperationRecorder is a thread-safe display list that transfers drawing commands to worker threads.
However, WebCore::Pattern isn't a thread-safe object. So CairoOperationRecorder append a command by converting the object to cairo object.

[Radar WebKit Bug Importer]

<rdar://problem/80818847>

[Fujii Hironori]

Created attachment 434192 [details]
Patch