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

Issue 11361129: Don't invert contentsDeviceTransform twice when rendering render passes. (Closed)

Created:
8 years, 1 month ago by slavi
Modified:
8 years, 1 month ago
Reviewers:
danakj, jamesr, enne (OOO)
CC:
chromium-reviews, cc-bugs_chromium.org
Visibility:
Public.

Description

Don't invert contentsDeviceTransform twice when rendering render passes. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=166586

Patch Set 1 #

Total comments: 3

Patch Set 2 : Addressing code reviews. #

Patch Set 3 : Rebase. #

Patch Set 4 : Matrix->Transform #

Total comments: 1

Patch Set 5 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -10 lines) Patch
M cc/gl_renderer.h View 1 2 3 1 chunk +4 lines, -1 line 0 comments Download
M cc/gl_renderer.cc View 1 2 3 6 chunks +15 lines, -8 lines 0 comments Download
M cc/software_renderer.cc View 1 2 3 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 14 (0 generated)
slavi
8 years, 1 month ago (2012-11-06 19:19:29 UTC) #1
jamesr
http://codereview.chromium.org/11361129/diff/1/cc/gl_renderer.cc File cc/gl_renderer.cc (right): http://codereview.chromium.org/11361129/diff/1/cc/gl_renderer.cc#newcode518 cc/gl_renderer.cc:518: WebTransformationMatrix deviceMatrix = (frame.windowMatrix * frame.projectionMatrix * quadRectMatrix).to2dTransform(); what's ...
8 years, 1 month ago (2012-11-06 23:53:43 UTC) #2
shawnsingh
Personally I think it's better to keep the original name, it makes it clear (for ...
8 years, 1 month ago (2012-11-07 00:05:15 UTC) #3
slavi
On 2012/11/06 23:53:43, jamesr wrote: > http://codereview.chromium.org/11361129/diff/1/cc/gl_renderer.cc > File cc/gl_renderer.cc (right): > > http://codereview.chromium.org/11361129/diff/1/cc/gl_renderer.cc#newcode518 > ...
8 years, 1 month ago (2012-11-07 00:31:40 UTC) #4
danakj
On Tue, Nov 6, 2012 at 7:31 PM, <skaslev@chromium.org> wrote: > On 2012/11/06 23:53:43, jamesr ...
8 years, 1 month ago (2012-11-07 00:37:37 UTC) #5
slavi
PTAL The Transform -> Matrix is consistent with matrix naming in DrawingFrame: http://code.google.com/searchframe#OAMlx_jo-ck/src/cc/direct_renderer.h&exact_package=chromium&q=DrawingFrame&type=cs&l=46
8 years, 1 month ago (2012-11-07 01:02:14 UTC) #6
danakj
It's funny, because the various renderers are the only place left where we have Matrix ...
8 years, 1 month ago (2012-11-07 01:10:40 UTC) #7
slavi
Then Transform it is. Coming from a gamedev background, Transform makes sense rather than Matrix. ...
8 years, 1 month ago (2012-11-07 01:31:25 UTC) #8
danakj
lgtm but bots hate me still. so you'll need enne/jamesr, or tbr one of them ...
8 years, 1 month ago (2012-11-07 01:35:18 UTC) #9
jamesr
On 2012/11/07 01:31:25, slavi wrote: > Then Transform it is. > > Coming from a ...
8 years, 1 month ago (2012-11-07 01:37:37 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/skaslev@chromium.org/11361129/8
8 years, 1 month ago (2012-11-07 01:45:37 UTC) #11
commit-bot: I haz the power
Step "update" is always a major failure. Look at the try server FAQ for more ...
8 years, 1 month ago (2012-11-07 04:13:03 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/skaslev@chromium.org/11361129/15003
8 years, 1 month ago (2012-11-07 18:42:28 UTC) #13
commit-bot: I haz the power
8 years, 1 month ago (2012-11-08 02:21:48 UTC) #14
Change committed as 166586

Powered by Google App Engine
This is Rietveld 408576698