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

Issue 5486054: two loops for setting stage descs (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 9 months ago by bsalomon
Modified:
12 years, 8 months ago
Reviewers:
TomH
Base URL:
http://skia.googlecode.com/svn/trunk/
Visibility:
Public.

Patch Set 1 #

Patch Set 2 : cleanup #

Unified diffs Side-by-side diffs Delta from patch set Stats (+134 lines, -108 lines) Patch
M src/gpu/GrGLProgram.h View 1 chunk +3 lines, -0 lines 0 comments Download
M src/gpu/GrGpuGL.h View 1 5 chunks +14 lines, -13 lines 0 comments Download
M src/gpu/GrGpuGLShaders.cpp View 3 chunks +117 lines, -95 lines 0 comments Download

Messages

Total messages: 3
bsalomon
tiny perf gain, but seems nicer to me.
12 years, 9 months ago (2011-12-13 21:18:51 UTC) #1
TomH
Yes, like the refactoring. LGTM. What's the perf gain coming from - was there a ...
12 years, 9 months ago (2011-12-14 15:18:25 UTC) #2
bsalomon
12 years, 9 months ago (2011-12-14 15:33:19 UTC) #3
On 2011/12/14 15:18:25, TomH wrote:
> Yes, like the refactoring. LGTM.
> What's the perf gain coming from - was there a rewrite in the code that got
> pulled out into the new function? If so, it wasn't easy to spot; the logic for
> what gets copied & what gets skipped looks equivalent...

Now I'm not sure if there really was a perf gain at all given the flakiness of
the bench runs. But it looked like there were some extra store/loads around the
stage enabled bool and also in determining whether to check skipColor or
skipCoverage based on the stage index and desc.firstCoverageStage(). In any
event I'm confident the performance isn't worse and I think it is an improvement
to the code.
Sign in to reply to this message.

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