Bug 59046 - With a non-1 page scale, scrolling to reveal selection fails
Summary: With a non-1 page scale, scrolling to reveal selection fails
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: mitz
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-04-20 17:17 PDT by mitz
Modified: 2011-04-20 20:02 PDT (History)
3 users (show)

See Also:


Attachments
Patch (5.88 KB, patch)
2011-04-20 18:57 PDT, mitz
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2011-04-20 17:17:19 PDT
With a non-1 page scale, scrolling to reveal selection fails
Comment 1 mitz 2011-04-20 18:55:07 PDT
<rdar://problem/9095366>
Comment 2 mitz 2011-04-20 18:57:04 PDT
Created attachment 90474 [details]
Patch
Comment 3 Maciej Stachowiak 2011-04-20 19:06:10 PDT
Comment on attachment 90474 [details]
Patch

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

r=me

> Source/WebCore/rendering/RenderObject.cpp:1164
>          if (!v->hasLayer() || !v->layer()->isComposited() || v->layer()->backing()->paintingGoesToWindow()) {
> -            v->repaintViewRectangle(r, immediate);
> +            IntRect repaintRectangle = r;
> +            if (v->hasLayer() &&  v->layer()->transform() && v->layer()->isComposited() && v->layer()->backing()->paintingGoesToWindow())
> +                repaintRectangle = v->layer()->transform()->mapRect(r);
> +            v->repaintViewRectangle(repaintRectangle, immediate);

The logic here is a bit squarely. You test some conditions and then similar or opposite looking ones right inside that if. I wonder if this can be refactored to be more clear, though I have no specific suggestions.
Comment 4 mitz 2011-04-20 19:11:19 PDT
(In reply to comment #3)
> (From update of attachment 90474 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=90474&action=review
> 
> r=me
> 
> > Source/WebCore/rendering/RenderObject.cpp:1164
> >          if (!v->hasLayer() || !v->layer()->isComposited() || v->layer()->backing()->paintingGoesToWindow()) {
> > -            v->repaintViewRectangle(r, immediate);
> > +            IntRect repaintRectangle = r;
> > +            if (v->hasLayer() &&  v->layer()->transform() && v->layer()->isComposited() && v->layer()->backing()->paintingGoesToWindow())
> > +                repaintRectangle = v->layer()->transform()->mapRect(r);
> > +            v->repaintViewRectangle(repaintRectangle, immediate);
> 
> The logic here is a bit squarely. You test some conditions and then similar or opposite looking ones right inside that if. I wonder if this can be refactored to be more clear, though I have no specific suggestions.

I can (and will) remove the “ &&  v->layer()->backing()->paintingGoesToWindow()” part, because it’s redundant, and add a local boolean variable for v->hasLayer() && v->layer()->isComposited()
Comment 5 mitz 2011-04-20 19:14:02 PDT
Fixed in r84454. <http://trac.webkit.org/changeset/84454>
Comment 6 WebKit Review Bot 2011-04-20 20:02:17 PDT
http://trac.webkit.org/changeset/84454 might have broken Qt Linux Release
The following tests are not passing:
fast/transforms/selection-bounds-in-transformed-view.html