Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(82)

Issue 2143143002: Add Texture2D and Sampler GrSLTypes (Closed)

Created:
4 years, 5 months ago by egdaniel
Modified:
4 years, 5 months ago
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/skia.git@master
Target Ref:
refs/heads/master
Project:
skia
Visibility:
Public.

Description

Add Texture2D and Sampler GrSLTypes These two new types are in support of Vulkan and the ability to send separate texture and sampler uniforms to the shader. They don't really fit well in the current system, since the current system ties together to idea of intended use and how to emit shader code into the same GrSLType enum. In vulkan, I want the GrGLSLSampler object to be used as a Sampler2D, but when appending its declaration it will emit a Texture2D and sampler object. Our query for GrSLTypeIsSamplerType refers more to the combination of texture and sampler and not just the sampler part. The GrSLTypeIs2DTextureType query is for is a a SamplerType that uses Texture2Ds. My new types don't really fit into either these categories as they are just half of the whole. In some refactoring down the road (possibly connected with SkSL), I suggest we split apart the concept of how we intend to use a GrGLSLSampler (Sampler2D, SamplerBuffer, etc.), from how we actually add it to the code (sampler, texture2D, sampler2D, etc.). BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2143143002 Committed: https://skia.googlesource.com/skia/+/990dbc88796f656418bcc4c196df30ed9bef6345

Patch Set 1 #

Total comments: 3

Patch Set 2 : update enum names #

Patch Set 3 : nits #

Unified diffs Side-by-side diffs Delta from patch set Stats (+120 lines, -91 lines) Patch
M include/gpu/GrTypesPriv.h View 1 8 chunks +55 lines, -43 lines 0 comments Download
M src/gpu/gl/GrGLGpu.cpp View 1 2 5 chunks +12 lines, -8 lines 0 comments Download
M src/gpu/gl/GrGLProgramDesc.cpp View 1 2 2 chunks +4 lines, -4 lines 0 comments Download
M src/gpu/gl/GrGLSampler.h View 1 1 chunk +1 line, -1 line 0 comments Download
M src/gpu/gl/GrGLTexture.cpp View 1 1 chunk +3 lines, -3 lines 0 comments Download
M src/gpu/glsl/GrGLSL.h View 1 2 4 chunks +12 lines, -7 lines 0 comments Download
M src/gpu/glsl/GrGLSLProgramBuilder.cpp View 1 2 chunks +2 lines, -2 lines 0 comments Download
M src/gpu/glsl/GrGLSLSampler.h View 1 1 chunk +1 line, -1 line 0 comments Download
M src/gpu/glsl/GrGLSLShaderBuilder.cpp View 1 2 chunks +2 lines, -2 lines 0 comments Download
M src/gpu/glsl/GrGLSLUniformHandler.h View 1 2 chunks +2 lines, -2 lines 0 comments Download
M src/gpu/vk/GrVkGLSLSampler.h View 1 1 chunk +1 line, -1 line 0 comments Download
M src/gpu/vk/GrVkTexture.cpp View 1 3 chunks +3 lines, -3 lines 0 comments Download
M src/gpu/vk/GrVkUniformHandler.cpp View 1 5 chunks +22 lines, -14 lines 0 comments Download

Messages

Total messages: 12 (4 generated)
egdaniel
4 years, 5 months ago (2016-07-12 19:10:53 UTC) #3
bsalomon
https://codereview.chromium.org/2143143002/diff/1/include/gpu/GrTypesPriv.h File include/gpu/GrTypesPriv.h (right): https://codereview.chromium.org/2143143002/diff/1/include/gpu/GrTypesPriv.h#newcode29 include/gpu/GrTypesPriv.h:29: kSamplerExternal_GrSLType, I think with the addition of kSampler_GrSLType these ...
4 years, 5 months ago (2016-07-12 19:47:56 UTC) #4
egdaniel
https://codereview.chromium.org/2143143002/diff/1/include/gpu/GrTypesPriv.h File include/gpu/GrTypesPriv.h (right): https://codereview.chromium.org/2143143002/diff/1/include/gpu/GrTypesPriv.h#newcode29 include/gpu/GrTypesPriv.h:29: kSamplerExternal_GrSLType, On 2016/07/12 19:47:56, bsalomon wrote: > I think ...
4 years, 5 months ago (2016-07-12 20:34:11 UTC) #5
egdaniel
https://codereview.chromium.org/2143143002/diff/1/include/gpu/GrTypesPriv.h File include/gpu/GrTypesPriv.h (right): https://codereview.chromium.org/2143143002/diff/1/include/gpu/GrTypesPriv.h#newcode29 include/gpu/GrTypesPriv.h:29: kSamplerExternal_GrSLType, On 2016/07/12 20:34:11, egdaniel wrote: > On 2016/07/12 ...
4 years, 5 months ago (2016-07-12 20:34:52 UTC) #6
egdaniel
ping
4 years, 5 months ago (2016-07-13 20:27:20 UTC) #7
bsalomon
lgtm
4 years, 5 months ago (2016-07-13 20:33:15 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2143143002/40001
4 years, 5 months ago (2016-07-13 20:35:44 UTC) #10
commit-bot: I haz the power
4 years, 5 months ago (2016-07-13 21:09:33 UTC) #12
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/990dbc88796f656418bcc4c196df30ed9bef6345

Powered by Google App Engine
This is Rietveld 408576698