Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
912483004:
AA stroke rects batch (Closed)
|
Created:
5 years, 10 months ago by joshua.litt Modified:
5 years, 10 months ago CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia.git@oval Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionAA stroke rects batch
BUG=skia:
Committed: https://skia.googlesource.com/skia/+/68c7b6aa98816badbbb69ef65a84a932b8dd58f3
Committed: https://skia.googlesource.com/skia/+/b0fb935bd544b0c6f68c692c7e1ee40876184a1a
Committed: https://skia.googlesource.com/skia/+/4283f13af2bb12fbc2856de9a730038a8ca1f2d7
Patch Set 1 #Patch Set 2 : cleanup #
Total comments: 5
Patch Set 3 : feedback inc #Patch Set 4 : comment tweak #Patch Set 5 : more little cleanups #Patch Set 6 : fix windows bug #Patch Set 7 : ignoring assert for now #Patch Set 8 : one cl and rebase #Messages
Total messages: 24 (10 generated)
joshualitt@google.com changed reviewers: + bsalomon@google.com, joshualitt@google.com
I think it is time for someone else to start looking at these CLs. We need everyone else to get familiar with the model and should have more eyes on it.
joshualitt@google.com changed reviewers: + robertphillips@google.com
On 2015/02/11 19:08:18, bsalomon wrote: > I think it is time for someone else to start looking at these CLs. We need > everyone else to get familiar with the model and should have more eyes on it. Robert, could you please take a look?
https://codereview.chromium.org/912483004/diff/20001/src/gpu/GrAARectRenderer... File src/gpu/GrAARectRenderer.cpp (right): https://codereview.chromium.org/912483004/diff/20001/src/gpu/GrAARectRenderer... src/gpu/GrAARectRenderer.cpp:546: struct Geometry { Line these guys up? That's a lot of data for a single AA rect :( Is there any way we could just store the viewmatrix (and maybe the miter flag) once per batch? The outsideAssist rect is only used when !mitering. https://codereview.chromium.org/912483004/diff/20001/src/gpu/GrAARectRenderer... src/gpu/GrAARectRenderer.cpp:607: SkAutoTUnref ? https://codereview.chromium.org/912483004/diff/20001/src/gpu/GrAARectRenderer... src/gpu/GrAARectRenderer.cpp:684: can this be const? https://codereview.chromium.org/912483004/diff/20001/src/gpu/GrAARectRenderer... src/gpu/GrAARectRenderer.cpp:748: Hmm - do we still need this guard? https://codereview.chromium.org/912483004/diff/20001/src/gpu/GrAARectRenderer... src/gpu/GrAARectRenderer.cpp:839: struct BatchTracker { Line these guys up?
After in-person conversation, lgtm with the addition of a bunch of TODOs.
New patchsets have been uploaded after l-g-t-m from robertphillips@google.com
The CQ bit was checked by joshualitt@google.com
The CQ bit was unchecked by joshualitt@google.com
The CQ bit was checked by joshualitt@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/912483004/80001
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://skia.googlesource.com/skia/+/68c7b6aa98816badbbb69ef65a84a932b8dd58f3
Message was sent while issue was closed.
A revert of this CL (patchset #5 id:80001) has been created in https://codereview.chromium.org/921923002/ by joshualitt@google.com. The reason for reverting is: breaks windows.
The CQ bit was checked by joshualitt@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/912483004/100001
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as https://skia.googlesource.com/skia/+/b0fb935bd544b0c6f68c692c7e1ee40876184a1a
Message was sent while issue was closed.
A revert of this CL (patchset #6 id:100001) has been created in https://codereview.chromium.org/924533004/ by joshualitt@google.com. The reason for reverting is: breaks in debug nanobench.
The CQ bit was checked by joshualitt@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/912483004/120001
The CQ bit was unchecked by joshualitt@google.com
The CQ bit was checked by joshualitt@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/912483004/140001
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as https://skia.googlesource.com/skia/+/4283f13af2bb12fbc2856de9a730038a8ca1f2d7 |
