[Webkit-unassigned] [Bug 98989] [CSS Shaders] Add CustomFilterRenderer to reuse this class by Accelerated Compositing.

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Huang Dongsung <luxtella at company100.net>  2012-10-16 21:45:25 PST ---
(In reply to comment #4)
> (From update of attachment 168138 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=168138&action=review

Thank you for your review!

> > 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.

Ok. I will make FECustomFilter have no dependency on CustomFilterOperation. I think it is better for FECustomFilter to not know CustomFilterOperation.
IMHO, each GraphicsLayer uses FilterOperation directly. Moreover, TextureMapperLayer and TextureMaperGL heavily use FilterOperation, too.


> > 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.

You're right. The name is poor. I'll more refactor CustomFilterRenderer for the Accelerated Pipeline.


> > 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.

Yes, I can and I want :)
I want to file and post another bug removing CustomFilterDrawType which this bug depends on.

I have a question.
There is the CustomFilterProgramType enumeration in CustomFilterProgramInfo.h. Is it also useless?

enum CustomFilterProgramType {
    PROGRAM_TYPE_NO_ELEMENT_TEXTURE,
    PROGRAM_TYPE_BLENDS_ELEMENT_TEXTURE
};

CustomFilterProgramType is set only in StyleResolver.

PassRefPtr<CustomFilterOperation> StyleResolver::createCustomFilterOperation(WebKitCSSFilterValue* filterValue)
{
    ...
    CustomFilterProgramType programType = PROGRAM_TYPE_NO_ELEMENT_TEXTURE;
    ...
        if (fragmentShaderOrMixFunction->isWebKitCSSMixFunctionValue()) {
            programType = PROGRAM_TYPE_BLENDS_ELEMENT_TEXTURE;
        ...
}

I don't understand why only mix(<uri> ..) needs the DOM element texture. I think just <uri> fragmentShader also needs the DOM element texture.
http://dvcs.w3.org/hg/FXTF/raw-file/tip/filters/index.html#fragmentShader-attribute

Could I remove CustomFilterProgramType also?

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