svgdotjs / svg.filter.js

A plugin for svg.js adding svg filters
MIT License
218 stars 52 forks source link

diffuseLighting filter #18

Open mcrossley opened 7 years ago

mcrossley commented 7 years ago

I'm stuck on the diffuseLighting filter.

This is what I am trying to achieve...

<filter id="df1">
    <feDiffuseLighting in="SourceGraphic" result="light" lighting-color="white">
        <fePointLight z="20" y="60" x="60"></fePointLight>
    </feDiffuseLighting>
    <feComposite in="SourceGraphic" in2="light" operator="arithmetic" k1="4" k2="0" k3="0" k4="0"></feComposite>
</filter>

The plugin seems to be missing the parameter for "lighting-color" (mentioned in passing in the very last sentence at the end of the relevant section in W3 document!), and I'm not sure how to apply the fePointLight child element.

Any pointers gratefully received!

Thanks Mark

Fuzzyma commented 7 years ago

@rdfriedl can you take a look at this please?

mcrossley commented 7 years ago

OK, figured it out!

object.filter(function (add) {
    var diff = add.diffuseLighting()
        .attr({'lighting-color': 'white'});

    var pLight = new SVG.PointLight(20, 20, 60);
    pLight.attr({
        pointsAtX: 20,
        pointsAtY: 20
    });
    diff.add(pLight);

    add.composite(add.source, diff, 'arithmetic')
        .attr({k1: 3, k2: 0, k3: 0, k4: 0});
});

As "lighting-color" appears to be the only other possible parameter for DiffuseLighting, maybe add it to the constructor?

hzrd149 commented 7 years ago

@mcrossley nice catch, I dont know how I missed that property.

@Fuzzyma how should we add this argument / attribute? (doc) because right now the arguments for the diffuseLighting and specularLighting filters are as follows

filter.diffuseLighting(surfaceScale, diffuseConstant, kernelUnitLength);
filter.specularLighting(surfaceScale, diffuseConstant, specularExponent, kernelUnitLength);

since they are somewhat important arguments for the lighting effects do you think its worth adding then to the beginning on the arguments?

// doing this would break the backwards compatibility of svg.filter.js
// but it would make setting the lighting-color easier
filter.diffuseLighting(surfaceScale, lightingColor, diffuseConstant, kernelUnitLength);
filter.specularLighting(surfaceScale, lightingColor, diffuseConstant, specularExponent, kernelUnitLength);

or should we just append then onto the end of the arguments?

// this would keep the backwards compatibility of svg.filter.js
// but it might make it incontinent to set the color, or it might confuse some people
filter.diffuseLighting(surfaceScale, diffuseConstant, kernelUnitLength, lightingColor);
filter.specularLighting(surfaceScale, diffuseConstant, specularExponent, kernelUnitLength, lightingColor);
Fuzzyma commented 7 years ago

I would say append them for now. When 3.0 is out we have to rewrite the plugins anyway. This change could be made there then

D4vx commented 5 years ago

I followed the example above but was using a point light source as a child of a specular lighting filter. var pLight = new SVG.PointLight(20, 20, 60)

This was throwing an error ..

TypeError: SVG.PointLight is not a constructor

I was able resolve this using the following syntax const pLight = SVG.create('fePointLight'); pLight.setAttribute("x", "20"); pLight.setAttribute("y", "20"); pLight.setAttribute("z", "60"); specularFilter.node.append(pLight);

This was then correctly applying the point light as a child of the specular light.

More of an FYI as this took me some time to find the correct information.