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

Issue 6343109: Fix lighting filters vs. flipY (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 11 months ago by Stephen White
Modified:
11 years, 11 months ago
Reviewers:
bsalomon, TomH
CC:
(pls use tomhudson at google), skia-review_googlegroups.com
Base URL:
http://skia.googlecode.com/svn/trunk/
Visibility:
Public.

Description

There were three different problems with lighting filters: 1) Texture offsets (fImageIncrement) have to be signed depending on whether the texture is "right way up" (texture upload) or "upside down" (render target), so the surface normals were coming out upside down. 2) Light normals have to y-negated on upload These two bugs were cancelling each other out in SampleApp, (where we were testing w/textures) but not in Chrome (where we were testing w/render targets). 3) The extract-the-height-from-the-view-matrix hack I was using to compare light positions vs. gl_FragCoord doesn't work in Chrome where we compile with GR_STATIC_RECT_VB, and the view matrix contains more than the viewport transform (to accomodate the canonical vertex buffer). Fixed by passing the destination render target to GrGLProgramStage::setData(), so it can flip the light positions in Y on the CPU.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+52 lines, -33 lines) Patch
src/effects/SkLightingImageFilter.cpp View 20 chunks +40 lines, -32 lines 0 comments Download
src/gpu/effects/GrConvolutionEffect.cpp View 2 chunks +2 lines, -0 lines 0 comments Download
src/gpu/effects/GrGradientEffects.cpp View 4 chunks +4 lines, -0 lines 0 comments Download
src/gpu/effects/GrMorphologyEffect.cpp View 2 chunks +2 lines, -0 lines 0 comments Download
src/gpu/gl/GrGLProgramStage.h View 1 chunk +1 line, -0 lines 0 comments Download
src/gpu/gl/GrGLProgramStage.cpp View 1 chunk +1 line, -0 lines 0 comments Download
src/gpu/gl/GrGpuGL_program.cpp View 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 4
Stephen White
11 years, 11 months ago (2012-07-13 18:11:20 UTC) #1
bsalomon
LGTM
11 years, 11 months ago (2012-07-13 18:16:06 UTC) #2
TomH
LGTM
11 years, 11 months ago (2012-07-13 18:21:23 UTC) #3
Stephen White
11 years, 11 months ago (2012-07-16 15:42:11 UTC) #4
On 2012/07/13 18:21:23, TomH wrote:
> LGTM

Landed as http://code.google.com/p/skia/source/detail?r=4605; closing.
Sign in to reply to this message.

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