Bug 49915 – [Qt] QWebFramePrivate::renderRelativeCoords() calls QPainter::save/restore more than necessary

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Andreas Kling]

Find a way to minimize the number of save/restore calls.

[Renata Hodovan]

Created attachment 79917 [details]
Patch

[Andreas Kling]

Comment on attachment 79917 [details]
Patch

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

> Source/WebKit/qt/ChangeLog:9
> +        In the first loop of renderRelativeCoords() the call of QPainter::save/restore is useless, because
> +        the only change is on it's context which is restored.

This sounds a bit strange, I would say something like "..., because the context is saved/restored within the loop."

> Source/WebKit/qt/ChangeLog:10
> +        In the second loop their calling is also avoidable by using invert translation on context.

s/their calling is/the calls are/
s/invert/inverse/

> Source/WebKit/qt/Api/qwebframe.cpp:408
>                  view->paintScrollbars(context, rect);

Are you sure that FrameView::paintScrollbars() doesn't taint the GraphicsContext state?

[Renata Hodovan]

(In reply to comment #2)
> (From update of attachment 79917 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=79917&action=review
> 
> > Source/WebKit/qt/ChangeLog:9
> > +        In the first loop of renderRelativeCoords() the call of QPainter::save/restore is useless, because
> > +        the only change is on it's context which is restored.
> 
> This sounds a bit strange, I would say something like "..., because the context is saved/restored within the loop."
> 
> > Source/WebKit/qt/ChangeLog:10
> > +        In the second loop their calling is also avoidable by using invert translation on context.
> 
> s/their calling is/the calls are/
> s/invert/inverse/
Uppppdated...

> > Source/WebKit/qt/Api/qwebframe.cpp:408
> >                  view->paintScrollbars(context, rect);
> 
> Are you sure that FrameView::paintScrollbars() doesn't taint the GraphicsContext state?
No, GC is absolutely undamaged :)

[Andreas Kling]

Comment on attachment 79917 [details]
Patch

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

>>> Source/WebKit/qt/Api/qwebframe.cpp:408
>>>                  view->paintScrollbars(context, rect);
>> 
>> Are you sure that FrameView::paintScrollbars() doesn't taint the GraphicsContext state?
> 
> No, GC is absolutely undamaged :)

Awesomecake! r=me

[Renata Hodovan]

Closing bug.
This is landed in <http://trac.webkit.org/changeset/76516>

[WebKit Review Bot]

http://trac.webkit.org/changeset/76516 might have broken GTK Linux 32-bit Debug
The following tests are not passing:
editing/selection/extend-selection-bidi.html