[Webkit-unassigned] [Bug 39582] [Gtk] very slow page scrolling on big -webkit-box-shadow areas

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=39582





--- Comment #22 from Alejandro G. Castro <alex at igalia.com>  2010-08-12 05:42:56 PST ---

Thanks for the detailed review.

(In reply to comment #20)
>
> [...]
>
> Can you add a better naming schema? I have no idea what ResultBufferOp means and Blit or Relase are not very helpful (I even don't know what Blit means :-P)
> 

Yeah, naming is always the hardest, it tried to be Result Buffer Operation, I know, it sucks :). http://en.wikipedia.org/wiki/Bit_blit. I'll try to get something better.

>
> [...]
>
> > +}
> This is pretty hard to understand. first,there shouldn't be that much code in an if clause. I suggest to test for the else, move the content of the else in this if and return earlier. the rest don't need to be checked again:
> 
> > +        tileRect.setX(0);
> > +        tileRect.setY(smallBufferSize.height() - 2 * radius - bottomRightRadius.height());
> > +        tileRect.setWidth(2 * radius + bottomLeftRadius.width());
> > +        tileRect.setHeight(2 * radius + bottomLeftRadius.height());
> This should be set at once:
>    tileRect = FloatRect(0, smallBufferSize.height() - 2 * radius - bottomRightRadius.height(), ...)
> Of course you can use multiple line for that.
> 
> You should use the logic of FloatRect/Size/Point much more in your code. and also it's funtcions. E.g. inflateX and inflateY extends the rect. Please use them to enlarge your rect to respect the blurring.
> Don't mix up the enlargement of your rects with the offset. You can apply the blur radius to the with and position of the rects and move the rect by the shadowOffset.
> Some multiplications are done multiple times (radius * 2) can you store them in a new variable like radiusTwice and so on?
> 
> It's great that you try to reuse the drawPattern code.
> 
> WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:1303
>  +          if (offsetHeight > min (bottomLeftRadius.height(), bottomRightRadius.height()))
> no space between min (
> 
> WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:1296
>  +      if (offsetWidth > 0)
> use if (offsetWidth)
> The same in other lines.

I agree, good points, I'll check them.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.