Open Bug 787059 Opened 12 years ago Updated 2 years ago

Duplicated RGBA External shader program

Categories

(Core :: Graphics: Layers, defect)

defect

Tracking

()

People

(Reporter: kanru, Unassigned)

Details

Attachments

(1 file)

Currently we have two such shader

 gl::RGBALayerExternalProgramType
 gl::RGBAExternalLayerProgramType

which have only slightly difference. We should be able to merge them.
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-

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Assignee: kanru → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: