[Qt] All widgets are rendered incorrectly when rendered through a cache

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Benjamin Poulain]

Created attachment 70952 [details]
Sample widget showing the problem

When QtWebKit render widgets on a pixmap instead of a QWidget, the widgets are rendered without a correct QStyleOption. This makes the widgets looks incorrect in many cases. For example, all widgets are rendered with a disabled state (with accelerated compositing or with tiling).

The problem is that we initialize the QStyleOption from the widget the view is rendered on. This widget is obtained from the graphics context:
    QPaintDevice* dev = 0;
    if (painter)
        dev = painter->device();
    if (dev && dev->devType() == QInternal::Widget)
        widget = static_cast<QWidget*>(dev);

This is broken with caches because the type is pixmap, not widget.

We could fix the issue by either:
1) introduce a target widget in PaintInfo
2) initialize the QStyleOption with sensible default value

There is the use case where 2 graphics view show the same scene, one graphics view is enabled, the second is disabled. This can only be solved by (1) + flushing all caches all the time when there is more than one view.

I am in favor of using (2) even if it is incorrect. This means the widgets would be rendered as active even if the QGraphicsView is in disabled state. I prefer this approach because I don't see a reasonable way to make it works when two graphics view show the same scene.
The resulting bug would be that widgets always appear enabled, even when rendered through a disabled QGraphicsView.

Input welcome :)

[Kenneth Rohde Christiansen]

(2) seems very fine with me. We really cannot support various views in a nice fashion as WebKit was not designed with this in mind, and it already breaks with Windowed plugins etc. (Just look at our PageClient implementations :( )

[Noam Rosenthal]

What is the user story for this?

[Noam Rosenthal]

(In reply to comment #2)
> What is the user story for this?

I'm in favor of (2) as well, just hard to me to understand why this is a big problem - having a good user story for this would let us understand the right track.

[Kenneth Rohde Christiansen]

I guess it is the following:

With QWebView the QStyle comes from the active QWebView, when using tiling etc, we paint to images, thus they have no access to an active QWidget, thus they get wrong style info and paint wrongly.

[Benjamin Poulain]

Created attachment 71437 [details]
Patch

[Simon Hausmann]

Comment on attachment 71437 [details]
Patch

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

> WebCore/platform/qt/RenderThemeQt.cpp:92
> +    if (widget)
> +        option.initFrom(widget);
> +    else
> +        /*
> +          If a widget is not directly available for rendering, we fallback to default
> +          value for an active widget.
> +         */
> +        option.state = QStyle::State_Active | QStyle::State_Enabled;

From the style guide: "One-line control clauses should not use braces unless comments are included or a single statement spans multiple lines." So curly braces are needed here.

The rest of the patch looks great! r+, but please this style buglet before landing :)

[Benjamin Poulain]

Committed r70297: <http://trac.webkit.org/changeset/70297>