Bug 49915 - [Qt] QWebFramePrivate::renderRelativeCoords() calls QPainter::save/restore more than necessary
Summary: [Qt] QWebFramePrivate::renderRelativeCoords() calls QPainter::save/restore mo...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Renata Hodovan
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2010-11-22 09:21 PST by Andreas Kling
Modified: 2011-01-24 08:46 PST (History)
3 users (show)

See Also:


Attachments
Patch (2.24 KB, patch)
2011-01-24 06:19 PST, Renata Hodovan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2010-11-22 09:21:13 PST
Find a way to minimize the number of save/restore calls.
Comment 1 Renata Hodovan 2011-01-24 06:19:46 PST
Created attachment 79917 [details]
Patch
Comment 2 Andreas Kling 2011-01-24 06:24:08 PST
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?
Comment 3 Renata Hodovan 2011-01-24 06:49:11 PST
(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 :)
Comment 4 Andreas Kling 2011-01-24 06:53:01 PST
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
Comment 5 Renata Hodovan 2011-01-24 07:03:04 PST
Closing bug.
This is landed in <http://trac.webkit.org/changeset/76516>
Comment 6 WebKit Review Bot 2011-01-24 08:46:54 PST
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