Duplicated RGBA External shader program

NEW
Assigned to

Status

()

Core
Graphics: Layers
5 years ago
5 years ago

People

(Reporter: kanru, Assigned: kanru)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Currently we have two such shader

 gl::RGBALayerExternalProgramType
 gl::RGBAExternalLayerProgramType

which have only slightly difference. We should be able to merge them.
(Assignee)

Comment 1

5 years ago
Created attachment 659201 [details] [diff] [review]
Merge RGBALayerExternalProgramType and RGBAExternalLayerProgramType.
Attachment #659201 - Flags: review?(jgilbert)
Attachment #659201 - Flags: feedback?(snorp)
Comment on attachment 659201 [details] [diff] [review]
Merge RGBALayerExternalProgramType and RGBAExternalLayerProgramType.

Review of attachment 659201 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, assuming you caught all the usages of both shaders.
Attachment #659201 - Flags: feedback?(snorp) → feedback+
Comment on attachment 659201 [details] [diff] [review]
Merge RGBALayerExternalProgramType and RGBAExternalLayerProgramType.

Review of attachment 659201 [details] [diff] [review]:
-----------------------------------------------------------------

I think for performance reasons, we should keep both of these, but should just rename them to better differentiate them.

::: gfx/layers/opengl/LayerManagerOGLShaders.h
@@ +362,5 @@
>  void main()\n\
>  {\n\
>  float mask = 1.0;\n\
>  \n\
> +gl_FragColor = texture2D(uTexture, (uTextureTransform * vec4(vTexCoord.x, vTexCoord.y, 0.0, 1.0)).xy) * uLayerOpacity * mask;\n\

What worries me here is that this becomes a dynamic texture lookup, whereas before we use vTexCoord directly. This can have adverse performance characteristics, since with static texture lookup, it can prefetch texels prior to fragment shader computation.

Also, a |mat4 * vec4| operation is an expensive thing to do in a fragment shader, and we should skip it if at all possible.
Attachment #659201 - Flags: review?(jgilbert) → review-
You need to log in before you can comment on or make changes to this bug.