Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(68)

Issue 6345081: GPU implementation of lighting filters (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years ago by Stephen White
Modified:
13 years ago
Reviewers:
reed1, bsalomon, TomH
Base URL:
http://skia.googlecode.com/svn/trunk/
Visibility:
Public.

Description

This patch implements the diffuse and specular lighting filters in Ganesh. There are three light types for each: distant, point and spot, whose code generation lives in a GrGLLight class hierarchy. This similar to the CPU implementation, where each light type provides a function to compute the vector from the surface plane to the light (surfaceToLight) and to compute the light colour (emitLightColour). Instead of templated member functions, as in the CPU implementation, these are virtual functions to emit the light-specific GLSL code. All of the code for the GPU path lives in the same file as that for the CPU path, SkLightingImageFilter.cpp. In order to provide Ganesh a hook to access it, SkImageFilter now has a asNewCustomStage() virtual, which allows an image filter to return a GrCustomStage representing that filter. Note that this patch does not handle the border conditions correctly (the [top|bottom][Left|Right]Normal() functions in the CPU implementation). That will come in a future patch.

Patch Set 1 #

Patch Set 2 : Fix for SK_SCALAR_IS_FIXED build #

Total comments: 6

Patch Set 3 : Interim version, to fix the GrGLLight singleton problem #

Patch Set 4 : Implemented asCustomStage(), moved all effect code to SkLightImageFilter #

Total comments: 4

Patch Set 5 : Move applyCustomStage into SkGpuDevice.cpp; improve comments #

Patch Set 6 : Add test cases to GrGpuGL_unittest.cpp #

Total comments: 1

Patch Set 7 : asNewCustomStage(), GrStringBuilder -> SkString, _VariableLifetime -> _ShaderType #

Unified diffs Side-by-side diffs Delta from patch set Stats (+883 lines, -4 lines) Patch
gyp/effects.gyp View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download
gyp/gpu.gyp View 1 2 3 4 5 6 1 chunk +3 lines, -0 lines 0 comments Download
include/core/SkImageFilter.h View 1 2 3 4 5 6 2 chunks +12 lines, -0 lines 0 comments Download
include/effects/SkLightingImageFilter.h View 1 2 3 4 5 6 1 chunk +3 lines, -0 lines 0 comments Download
include/gpu/GrProgramStageFactory.h View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
src/core/SkPaint.cpp View 1 2 3 4 5 6 1 chunk +4 lines, -0 lines 0 comments Download
src/effects/SkLightingImageFilter.cpp View 1 2 3 4 5 6 17 chunks +728 lines, -2 lines 0 comments Download
src/gpu/SkGpuDevice.cpp View 1 2 3 4 5 6 3 chunks +39 lines, -2 lines 0 comments Download
src/gpu/gl/GrGpuGL_unittest.cpp View 1 2 3 4 5 6 4 chunks +91 lines, -0 lines 0 comments Download

Messages

Total messages: 31
Stephen White
13 years ago (2012-07-10 14:33:46 UTC) #1
Stephen White
Fix for SK_SCALAR_IS_FIXED build
13 years ago (2012-07-10 14:55:59 UTC) #2
bsalomon
It seems like a problem that the GrGLLight instances are singeltons given that the GrGLProgramStage ...
13 years ago (2012-07-10 15:34:23 UTC) #3
TomH
I think we're going to have to do a lot of work to be happy ...
13 years ago (2012-07-10 15:47:59 UTC) #4
Stephen White
On 2012/07/10 15:34:23, bsalomon wrote: > It seems like a problem that the GrGLLight instances ...
13 years ago (2012-07-10 15:51:48 UTC) #5
bsalomon
Let's make src/effects able to see into include/gpu (and include/gpu/gl). We should then be able ...
13 years ago (2012-07-10 16:21:31 UTC) #6
Stephen White
> I think we're going to have to do a lot of work to be ...
13 years ago (2012-07-10 17:28:10 UTC) #7
Stephen White
Interim version, to fix the GrGLLight singleton problem
13 years ago (2012-07-10 17:29:42 UTC) #8
TomH
On 2012/07/10 16:21:31, bsalomon wrote: > Also, we should go ahead with adding virtual asCustomStage() ...
13 years ago (2012-07-10 17:31:16 UTC) #9
TomH
On 2012/07/10 17:31:16, TomH wrote: > First version of the generic part of this under ...
13 years ago (2012-07-10 18:20:16 UTC) #10
Stephen White
Implemented asCustomStage(), moved all effect code to SkLightImageFilter
13 years ago (2012-07-10 19:26:06 UTC) #11
bsalomon
The asACustomStage is a lot better! http://codereview.appspot.com/6345081/diff/6004/include/core/SkImageFilter.h File include/core/SkImageFilter.h (right): http://codereview.appspot.com/6345081/diff/6004/include/core/SkImageFilter.h#newcode83 include/core/SkImageFilter.h:83: * not. If ...
13 years ago (2012-07-10 19:47:49 UTC) #12
Stephen White
http://codereview.appspot.com/6345081/diff/6004/include/core/SkImageFilter.h File include/core/SkImageFilter.h (right): http://codereview.appspot.com/6345081/diff/6004/include/core/SkImageFilter.h#newcode83 include/core/SkImageFilter.h:83: * not. If stage is non-NULL, a new GrCustomStage ...
13 years ago (2012-07-10 20:53:05 UTC) #13
Stephen White
Move applyCustomStage into SkGpuDevice.cpp; improve comments
13 years ago (2012-07-10 20:54:13 UTC) #14
bsalomon
On 2012/07/10 20:54:13, Stephen White wrote: > Move applyCustomStage into SkGpuDevice.cpp; improve comments One last ...
13 years ago (2012-07-10 21:10:42 UTC) #15
Stephen White
Add test cases to GrGpuGL_unittest.cpp
13 years ago (2012-07-10 21:40:15 UTC) #16
bsalomon
On 2012/07/10 21:40:15, Stephen White wrote: > Add test cases to GrGpuGL_unittest.cpp LGTM if the ...
13 years ago (2012-07-10 21:45:23 UTC) #17
reed1
I'm fine, modulo trying to make the name clearer that it is allocating a new ...
13 years ago (2012-07-11 13:57:02 UTC) #18
Stephen White
On 2012/07/11 13:57:02, reed1 wrote: > I'm fine, modulo trying to make the name clearer ...
13 years ago (2012-07-11 15:23:07 UTC) #19
reed1
On 2012/07/11 15:23:07, Stephen White wrote: > On 2012/07/11 13:57:02, reed1 wrote: > > I'm ...
13 years ago (2012-07-11 15:27:44 UTC) #20
reed1
13 years ago (2012-07-11 15:27:50 UTC) #21
Stephen White
On 2012/07/11 15:27:44, reed1 wrote: > On 2012/07/11 15:23:07, Stephen White wrote: > > On ...
13 years ago (2012-07-11 15:29:31 UTC) #22
Stephen White
On 2012/07/11 15:29:31, Stephen White wrote: > On 2012/07/11 15:27:44, reed1 wrote: > > On ...
13 years ago (2012-07-11 15:30:18 UTC) #23
bsalomon
On 2012/07/11 15:30:18, Stephen White wrote: > On 2012/07/11 15:29:31, Stephen White wrote: > > ...
13 years ago (2012-07-11 15:36:57 UTC) #24
bsalomon
On 2012/07/11 15:36:57, bsalomon wrote: > On 2012/07/11 15:30:18, Stephen White wrote: > > On ...
13 years ago (2012-07-11 15:37:16 UTC) #25
Stephen White
On 2012/07/11 15:37:16, bsalomon wrote: > On 2012/07/11 15:36:57, bsalomon wrote: > > On 2012/07/11 ...
13 years ago (2012-07-11 15:44:12 UTC) #26
reed1
On 2012/07/11 15:37:16, bsalomon wrote: > On 2012/07/11 15:36:57, bsalomon wrote: > > On 2012/07/11 ...
13 years ago (2012-07-11 15:44:15 UTC) #27
Stephen White
On 2012/07/11 15:44:15, reed1 wrote: > On 2012/07/11 15:37:16, bsalomon wrote: > > On 2012/07/11 ...
13 years ago (2012-07-11 15:53:13 UTC) #28
Stephen White
asNewCustomStage(), GrStringBuilder -> SkString, _VariableLifetime -> _ShaderType
13 years ago (2012-07-11 15:56:40 UTC) #29
bsalomon
On 2012/07/11 15:56:40, Stephen White wrote: > asNewCustomStage(), GrStringBuilder -> SkString, _VariableLifetime -> > _ShaderType ...
13 years ago (2012-07-11 15:58:49 UTC) #30
Stephen White
13 years ago (2012-07-12 20:24:42 UTC) #31
Closed with r4535.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b