Issue 13895006: Expand modulate, add, subtract, extract component glsl helpers.

Issue 13895006: Expand modulate, add, subtract, extract component glsl helpers. (Closed)

Created:
7 years, 8 months ago by bsalomon

Modified:
7 years, 8 months ago

Reviewers:
robertphillips

CC:
skia-review_googlegroups.com

Base URL:
http://skia.googlecode.com/svn/trunk/

Visibility:
Public.

More Reviews

Description

Expand modulate, add, subtract, extract component glsl helpers. Committed: https://code.google.com/p/skia/source/detail?r=8755

Patch Set 4 : #

Created: 7 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+508 lines, -340 lines)			Patch
M	gyp/gpu.gypi	View	1 2	1 chunk	+1 line, -0 lines	0 comments	Download
M	include/gpu/GrColor.h	View		1 chunk	+17 lines, -0 lines	0 comments	Download
M	include/gpu/GrTypesPriv.h	View		1 chunk	+2 lines, -0 lines	0 comments	Download
M	src/gpu/GrAAConvexPathRenderer.cpp	View		1 chunk	+1 line, -1 line	0 comments	Download
M	src/gpu/GrAAHairLinePathRenderer.cpp	View		2 chunks	+2 lines, -2 lines	0 comments	Download
M	src/gpu/GrAARectRenderer.cpp	View		1 chunk	+1 line, -1 line	0 comments	Download
M	src/gpu/GrOvalRenderer.cpp	View		2 chunks	+2 lines, -2 lines	0 comments	Download
M	src/gpu/effects/GrSimpleTextureEffect.cpp	View		1 chunk	+1 line, -1 line	0 comments	Download
M	src/gpu/gl/GrGLProgram.h	View	1	1 chunk	+3 lines, -3 lines	0 comments	Download
M	src/gpu/gl/GrGLProgram.cpp	View	1 2 3	9 chunks	+162 lines, -192 lines	0 comments	Download
M	src/gpu/gl/GrGLSL.h	View		4 chunks	+92 lines, -28 lines	0 comments	Download
M	src/gpu/gl/GrGLSL.cpp	View		2 chunks	+26 lines, -81 lines	0 comments	Download
A	src/gpu/gl/GrGLSL_impl.h	View	1 2	1 chunk	+193 lines, -0 lines	0 comments	Download
M	src/gpu/gl/GrGLShaderBuilder.cpp	View		2 chunks	+2 lines, -2 lines	0 comments	Download
M	src/gpu/gl/GrGLShaderVar.h	View		1 chunk	+3 lines, -27 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

bsalomon

This isn't a whole lot prettier but it reduces some of the complexity of writing ...

7 years, 8 months ago (2013-04-18 18:39:03 UTC) #1

robertphillips

lgtm + nits & some questions https://codereview.chromium.org/13895006/diff/20001/src/gpu/gl/GrGLProgram.cpp File src/gpu/gl/GrGLProgram.cpp (right): https://codereview.chromium.org/13895006/diff/20001/src/gpu/gl/GrGLProgram.cpp#newcode530 src/gpu/gl/GrGLProgram.cpp:530: SkString outCoverage; The ...

7 years, 8 months ago (2013-04-18 19:12:24 UTC) #2

bsalomon

Committed patchset #4 manually as r8755 (presubmit successful).

7 years, 8 months ago (2013-04-18 19:36:16 UTC) #3

bsalomon

7 years, 8 months ago (2013-04-18 19:36:37 UTC) #4

Message was sent while issue was closed.

https://codereview.chromium.org/13895006/diff/20001/src/gpu/gl/GrGLProgram.cpp
File src/gpu/gl/GrGLProgram.cpp (right):

https://codereview.chromium.org/13895006/diff/20001/src/gpu/gl/GrGLProgram.cp...
src/gpu/gl/GrGLProgram.cpp:530: SkString outCoverage;
On 2013/04/18 19:12:24, robertphillips wrote:
> The '&' seems odd here - as does having startStage at all.

Agreed, this was just moved from the old code.. not sure why it was written this
way. I just put the field in the loop init.

https://codereview.chromium.org/13895006/diff/20001/src/gpu/gl/GrGLProgram.cp...
src/gpu/gl/GrGLProgram.cpp:557: // discard if coverage is zero
On 2013/04/18 19:12:24, robertphillips wrote:
> constant on LHS?

Done.

https://codereview.chromium.org/13895006/diff/20001/src/gpu/gl/GrGLProgram.cp...
src/gpu/gl/GrGLProgram.cpp:672: if (bindDualSrcOut) {
On 2013/04/18 19:12:24, robertphillips wrote:
> ???

Oops, left in from debugging.

https://codereview.chromium.org/13895006/diff/20001/src/gpu/gl/GrGLSL.h
File src/gpu/gl/GrGLSL.h (right):

https://codereview.chromium.org/13895006/diff/20001/src/gpu/gl/GrGLSL.h#newco...
src/gpu/gl/GrGLSL.h:171: * Either can be NULL or "" in which case the default
params control whether a vector of ones or
On 2013/04/18 19:12:24, robertphillips wrote:
> if -> the? Here and in other two.

Done.

https://codereview.chromium.org/13895006/diff/20001/src/gpu/gl/GrGLSL.h#newco...
src/gpu/gl/GrGLSL.h:174: * or in1 will not be emitted (side effects won't
occur). The return value indicates whether a
On 2013/04/18 19:12:24, robertphillips wrote:
> is suppressed spelled correctly? Here and in other two.

Done.

https://codereview.chromium.org/13895006/diff/20001/src/gpu/gl/GrGLSL.h#newco...
src/gpu/gl/GrGLSL.h:186: /**
On 2013/04/18 19:12:24, robertphillips wrote:
> subtracting?

Done.

https://codereview.chromium.org/13895006/diff/20001/src/gpu/gl/GrGLSL.h#newco...
src/gpu/gl/GrGLSL.h:218: /**
On 2013/04/18 19:12:24, robertphillips wrote:
> If ...?

/**
 * Given an expression that evaluates to a GLSL vec4, extract a component. If
expr is NULL or ""
 * the value of defaultExpr is used. It is an error to pass an empty expr and
have set defaultExpr
 * to kNone. The return value indicates whether the value is known to be 0 or 1.
If omitIfConst is
 * set then nothing is appended when the return is not kNone.
 */

https://codereview.chromium.org/13895006/diff/20001/src/gpu/gl/GrGLSL_impl.h
File src/gpu/gl/GrGLSL_impl.h (right):

https://codereview.chromium.org/13895006/diff/20001/src/gpu/gl/GrGLSL_impl.h#...
src/gpu/gl/GrGLSL_impl.h:90: GrAssert(kNone_GrSLConstantVec != default1);
On 2013/04/18 19:12:24, robertphillips wrote:
> n -> numConstOnesVecs? -> resultOfSum?
"sum"

https://codereview.chromium.org/13895006/diff/20001/src/gpu/gl/GrGLSL_impl.h#...
src/gpu/gl/GrGLSL_impl.h:149: GrAssert(kNone_GrSLConstantVec != default1);
On 2013/04/18 19:12:24, robertphillips wrote:
> n -> resultOfDifference?
"diff"

Expand Messages | Collapse Messages