[Webkit-unassigned] [Bug 98989] [CSS Shaders] Add CustomFilterRenderer to reuse this class by Accelerated Compositing.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Oct 16 15:00:55 PDT 2012
https://bugs.webkit.org/show_bug.cgi?id=98989
--- Comment #4 from Chiculita Alexandru <achicu at adobe.com> 2012-10-16 15:01:46 PST ---
(From update of attachment 168138)
View in context: https://bugs.webkit.org/attachment.cgi?id=168138&action=review
Looks good! Thanks for the patch.
> Source/WebCore/ChangeLog:16
> + CustomFilterGlobalContext and CustomFilterOperation, and it is possible if we
I would remove the dependency on the CustomFilterOperation. Most probably the platform code has no access to it. Anyway, FECustomFilter had no dependency on that, so please remove it.
> Source/WebCore/ChangeLog:21
> + No new tests. Covered by css1/filters/custom
nit: typo css1 -> css3
> Source/WebCore/platform/graphics/filters/CustomFilterRenderer.cpp:55
> +PassRefPtr<CustomFilterRenderer> CustomFilterRenderer::create(CustomFilterGlobalContext* customFilterGlobalContext, CustomFilterOperation* operation)
This layer should not use the CustomFilterOperation directly. I would rather keep the same constructor as the FECustomFilter had before.
> Source/WebCore/platform/graphics/filters/CustomFilterRenderer.cpp:148
> +bool CustomFilterRenderer::render(Uint8ClampedArray* source, Uint8ClampedArray* destination, const IntSize& size)
I think this one could have a better name to explain that it would read back the pixels. I suppose at some point you will add one to execute only on textures, so that the Accelerated Pipeline doesn't read back.
> Source/WebCore/platform/graphics/filters/CustomFilterRenderer.cpp:474
> +static void orthogonalProjectionMatrix(TransformationMatrix& matrix, float left, float right, float bottom, float top)
> +{
> + ASSERT(matrix.isIdentity());
> +
I think static methods would better be placed at the beginning of the file.
> Source/WebCore/platform/graphics/filters/CustomFilterRenderer.h:67
> + enum CustomFilterDrawType {
> + NEEDS_INPUT_TEXTURE,
> + NO_INPUT_TEXTURE
I know you just copy-pasted this one, but it seems like NO_INPUT_TEXTURE is never used. Can you please remove CustomFilterDrawType type? I think it was intended to help implementing a renderer that has the input already as a texture instead of just some bytes in the memory.
For the accelerated compositor you will need a different render method that will take a source texture ID and a destination texture ID, so maybe we could add it back in a different form at that time.
> Source/WebCore/platform/graphics/filters/FECustomFilter.cpp:44
> +FECustomFilter::FECustomFilter(Filter* filter, PassRefPtr<CustomFilterRenderer> renderer)
I think FECustomFilter should create its own CustomFilterRenderer and not be passed one. CustomFilterRenderer would then be just an implementation detail.
--
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