Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(invert_filter): Alpha support #7933

Merged
merged 4 commits into from
Jun 10, 2022

Conversation

GM-Alex
Copy link
Contributor

@GM-Alex GM-Alex commented May 11, 2022

This implements a option flag for the invert filter to also invert the alpha values by the filter.

@ShaMan123 ShaMan123 changed the title Alpha support for invert feat(invert_filter): Alpha support May 12, 2022
@asturur
Copy link
Member

asturur commented May 14, 2022

h @GM-Alex this pr is welcome.
2 things:

  1. you shouldn't submit built dist folder when opening a pr.
  2. you can do this keeping a single shader and adding an extra line there. Do you think you can do that?

I can try to make an example if is not clear.
We can send an extra uniform exactly as uInvert, called uAlpha and add an if in the code.
Let me know what you think, should be less code overall.

@GM-Alex
Copy link
Contributor Author

GM-Alex commented May 14, 2022

@asturur The dist folder was added by accident. 🙈 I know how the shader code should look like but since I'm not familiar with the shader logic I don't know how to pass the right values to the shader code.

@asturur
Copy link
Member

asturur commented May 16, 2022

Let's do it together, if it looks nicer we keep it, or we revert back to this version.

@asturur
Copy link
Member

asturur commented May 16, 2022

At the end of the file you see this 2 function to which yuo can add your parameter

    /**
     * Return WebGL uniform locations for this filter's shader.
     *
     * @param {WebGLRenderingContext} gl The GL canvas context used to compile this filter's shader.
     * @param {WebGLShaderProgram} program This filter's compiled shader program.
     */
    getUniformLocations: function(gl, program) {
      return {
        uInvert: gl.getUniformLocation(program, 'uInvert'),
+       uAlpha: gl.getUnifromLocation(program, 'uAlpha'),
      };
    },
    /**
     * Send data from this filter to its shader program's uniforms.
     *
     * @param {WebGLRenderingContext} gl The GL canvas context used to compile this filter's shader.
     * @param {Object} uniformLocations A map of string uniform names to WebGLUniformLocation objects
     */
    sendUniformData: function(gl, uniformLocations) {
      gl.uniform1i(uniformLocations.uInvert, this.invert);
+     gl.uniform1i(uniformLocations.uAlpha, this.alpha);
    },

This would bring the uAlpha in the shader availabile.

At that point you can just pull it off in the original shader

    fragmentSource: 'precision highp float;\n' +
      'uniform sampler2D uTexture;\n' +
+     'uniform int uAlpha;\n' +
      'uniform int uInvert;\n' +
      'varying vec2 vTexCoord;\n' +
      'void main() {\n' +
        'vec4 color = texture2D(uTexture, vTexCoord);\n' +
        'if (uInvert == 1) {\n' +
          'gl_FragColor = vec4(1.0 - color.r,1.0 -color.g,1.0 -color.b,color.a);\n' +
        '} else {\n' +
          'gl_FragColor = color;\n' +
        '}\n' +
      '}',

And then you can decide how to change the rest.

To revert the extra dist folder, just run git checkout master dist

@GM-Alex
Copy link
Contributor Author

GM-Alex commented May 30, 2022

@asturur Sorry took me some time to get my hands on this again. Could you have a look again.

Comment on lines 64 to 75
* Retrieves the cached shader.
* @param {Object} options
* @param {WebGLRenderingContext} options.context The GL context used for rendering.
* @param {Object} options.programCache A map of compiled shader programs, keyed by filter type.
*/
retrieveShader: function(options) {
var cacheKey = this.type + '_' + (this.alpha ? 'alpha' : 'normal');
if (!options.programCache.hasOwnProperty(cacheKey)) {
options.programCache[cacheKey] = this.createProgram(options.context, this.fragmentSource);
}
return options.programCache[cacheKey];
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need this anymore now!
The shader is just one.

@asturur
Copy link
Member

asturur commented May 31, 2022

i looks better imho! There is an unused function now that can be removed.

@asturur asturur merged commit 4e7f332 into fabricjs:master Jun 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants