[Webkit-unassigned] [Bug 39582] [Gtk] very slow page scrolling on big -webkit-box-shadow areas
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Aug 12 05:31:19 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=39582
Dirk Schulze <krit at webkit.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #64004| |review-
Flag| |
--- Comment #20 from Dirk Schulze <krit at webkit.org> 2010-08-12 05:31:19 PST ---
(From update of attachment 64004)
A great patch, but maybe a bit more complicated than it should be.
> +#if PLATFORM(CAIRO)
> + // Decide if the shadow buffer should be blitted or released (required for tiling)
> + enum ResultBufferOp {
> + Blit,
> + Release
> + };
> +#endif
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)
> +/*
> + This function uses tiling to improve the performance of the shadow
> + drawing of rounded rectangles. The code basically does the following
> + steps:
> +
> + 1. Calculate the minimum rectangle size required to create the
> + tiles
> +
> + 2. If that size is smaller than the real rectangle render the new
> + small rectangle and its shadow, in other case render the shadow
> + of the real rectangle.
> +
> + 3. Calculate the sizes and positions of the tiles and their
> + destinations and use drawPattern to render the final shadow. The
> + code divides the rendering in 8 tiles:
> +
> + 1 | 2 | 3
> + -----------
> + 4 | | 5
> + -----------
> + 6 | 7 | 8
> +
> + The corners are directly copied from the small rectangle to the
> + real one and the side tiles are 1 pixel width, we use them as
> + tiles to cover the destination side. The corner tiles are bigger
> + than just the side of the rounded corner, we need to increase it
> + because the modifications caused by the corner over the blur
> + effect.
> + */
> +void GraphicsContext::drawFastShadow(GraphicsContext* context, GraphicsContextPrivate* gcp, const IntRect& r, const FloatSize& topLeftRadius, const FloatSize& topRightRadius, const FloatSize& bottomLeftRadius, const FloatSize& bottomRightRadius, ColorSpace colorSpace)
> +{
> +#if ENABLE(FILTERS)
> + FloatSize shadowSize;
> + FloatSize shadowOffset;
...
> + destRect.setX(shadowRect.x() + rect.width() - topRightRadius.width());
> + destRect.setY(shadowRect.y() + rect.height() - bottomRightRadius.height());
> + destRect.setWidth(tileRect.width());
> + destRect.setHeight(tileRect.height());
> + phase.setX(destRect.x() - tileRect.x());
> + phase.setY(destRect.y() - tileRect.y());
> +
> + shadowBuffer->image()->drawPattern(context, tileRect, patternTransform,
> + phase, colorSpace, CompositeSourceOver, destRect);
> +
> + // bottom left corner
> + tileRect.setX(0);
> + tileRect.setY(smallBufferSize.height() - 2 * radius - bottomRightRadius.height());
> + tileRect.setWidth(2 * radius + bottomLeftRadius.width());
> + tileRect.setHeight(2 * radius + bottomLeftRadius.height());
> + destRect.setX(shadowRect.x());
> + destRect.setY(shadowRect.y() + rect.height() - bottomRightRadius.height());
> + destRect.setWidth(tileRect.width());
> + destRect.setHeight(tileRect.height());
> + phase.setX(destRect.x() - tileRect.x());
> + phase.setY(destRect.y() - tileRect.y());
> +
> + shadowBuffer->image()->drawPattern(context, tileRect, patternTransform,
> + phase, colorSpace, CompositeSourceOver, destRect);
> +
> + } else {
> + // Create suitably-sized ImageBuffer to hold the shadow.
> + OwnPtr<ImageBuffer> shadowBuffer = ImageBuffer::create(shadowBufferSize);
> +
> + // Draw shadow into a new ImageBuffer.
> + cairo_t* shadowContext = shadowBuffer->context()->platformContext();
> + copyContextProperties(cr, shadowContext);
> + cairo_translate(shadowContext, -rect.x() + radius, -rect.y() + radius);
> + cairo_new_path(shadowContext);
> + cairo_append_path(shadowContext, path);
> +
> + setPlatformFill(context, shadowContext, gcp);
> +
> + context->createPlatformShadow(shadowBuffer.release(), shadowColor, shadowRect, radius, Blit);
> + }
> +#endif
> +}
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:
> + if (...) {
> + // Create suitably-sized ImageBuffer to hold the shadow.
> + OwnPtr<ImageBuffer> shadowBuffer = ImageBuffer::create(shadowBufferSize);
> +
> + // Draw shadow into a new ImageBuffer.
> + cairo_t* shadowContext = shadowBuffer->context()->platformContext();
> + copyContextProperties(cr, shadowContext);
> + cairo_translate(shadowContext, -rect.x() + radius, -rect.y() + radius);
> + cairo_new_path(shadowContext);
> + cairo_append_path(shadowContext, path);
> +
> + setPlatformFill(context, shadowContext, gcp);
> +
> + context->createPlatformShadow(shadowBuffer.release(), shadowColor, shadowRect, radius, Blit);
> return;
> + }
> ....
> + 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.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list