Open
Bug 787059
Opened 12 years ago
Updated 2 years ago
Duplicated RGBA External shader program
Categories
(Core :: Graphics: Layers, defect)
Core
Graphics: Layers
Tracking
()
NEW
People
(Reporter: kanru, Unassigned)
Details
Attachments
(1 file)
12.43 KB,
patch
|
jgilbert
:
review-
snorp
:
feedback+
|
Details | Diff | Splinter Review |
Currently we have two such shader gl::RGBALayerExternalProgramType gl::RGBAExternalLayerProgramType which have only slightly difference. We should be able to merge them.
Reporter | ||
Comment 1•12 years ago
|
||
Attachment #659201 -
Flags: review?(jgilbert)
Attachment #659201 -
Flags: feedback?(snorp)
Comment 2•12 years ago
|
||
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 3•12 years ago
|
||
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-
Comment 4•2 years ago
|
||
The bug assignee is inactive on Bugzilla, so the assignee is being reset.
Assignee: kanru → nobody
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•