Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
421593004:
Generalize compressed blitter into its own templated class (Closed)
|
Created:
6 years, 4 months ago by krajcevski Modified:
6 years, 4 months ago Reviewers:
robertphillips CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia.git@master Project:
skia Visibility:
Public. |
DescriptionGeneralize compressed blitter into its own templated class
Committed: https://skia.googlesource.com/skia/+/d5e46c7893afdd5976c1581a2ae81168252f5dec
Patch Set 1 #
Total comments: 14
Patch Set 2 : Code review changes #Patch Set 3 : Sync #
Messages
Total messages: 24 (0 generated)
lgtm + nits & questions https://codereview.chromium.org/421593004/diff/1/src/utils/SkTextureCompresso... File src/utils/SkTextureCompressor_Blitter.h (right): https://codereview.chromium.org/421593004/diff/1/src/utils/SkTextureCompresso... src/utils/SkTextureCompressor_Blitter.h:7: This needs a 'T' in its name to let everyone know it contains a templated class (e.g., SkTDArray). https://codereview.chromium.org/421593004/diff/1/src/utils/SkTextureCompresso... src/utils/SkTextureCompressor_Blitter.h:92: // assumptions, then it's time to flush them... 4? https://codereview.chromium.org/421593004/diff/1/src/utils/SkTextureCompresso... src/utils/SkTextureCompressor_Blitter.h:165: * Compressed texture blitters only really work correctly if they get four? https://codereview.chromium.org/421593004/diff/1/src/utils/SkTextureCompresso... src/utils/SkTextureCompressor_Blitter.h:225: blockColN ? https://codereview.chromium.org/421593004/diff/1/src/utils/SkTextureCompresso... src/utils/SkTextureCompressor_Blitter.h:257: don't ? https://codereview.chromium.org/421593004/diff/1/src/utils/SkTextureCompresso... src/utils/SkTextureCompressor_Blitter.h:299: // Setup: put sk_bzero's on their own line ? https://codereview.chromium.org/421593004/diff/1/src/utils/SkTextureCompresso... src/utils/SkTextureCompressor_Blitter.h:338: // Write this block The 'k' prefix is usually reserved for comments. I think template parameters usually just get a leading capital.
https://codereview.chromium.org/421593004/diff/1/src/utils/SkTextureCompresso... File src/utils/SkTextureCompressor_Blitter.h (right): https://codereview.chromium.org/421593004/diff/1/src/utils/SkTextureCompresso... src/utils/SkTextureCompressor_Blitter.h:7: On 2014/07/28 17:51:36, robertphillips wrote: > This needs a 'T' in its name to let everyone know it contains a templated class > (e.g., SkTDArray). Done. https://codereview.chromium.org/421593004/diff/1/src/utils/SkTextureCompresso... src/utils/SkTextureCompressor_Blitter.h:92: // assumptions, then it's time to flush them... On 2014/07/28 17:51:36, robertphillips wrote: > 4? Done. https://codereview.chromium.org/421593004/diff/1/src/utils/SkTextureCompresso... src/utils/SkTextureCompressor_Blitter.h:165: * Compressed texture blitters only really work correctly if they get On 2014/07/28 17:51:36, robertphillips wrote: > four? Done. https://codereview.chromium.org/421593004/diff/1/src/utils/SkTextureCompresso... src/utils/SkTextureCompressor_Blitter.h:225: On 2014/07/28 17:51:37, robertphillips wrote: > blockColN ? Done. https://codereview.chromium.org/421593004/diff/1/src/utils/SkTextureCompresso... src/utils/SkTextureCompressor_Blitter.h:257: On 2014/07/28 17:51:36, robertphillips wrote: > don't ? Done. https://codereview.chromium.org/421593004/diff/1/src/utils/SkTextureCompresso... src/utils/SkTextureCompressor_Blitter.h:299: // Setup: On 2014/07/28 17:51:37, robertphillips wrote: > put sk_bzero's on their own line ? Done. https://codereview.chromium.org/421593004/diff/1/src/utils/SkTextureCompresso... src/utils/SkTextureCompressor_Blitter.h:338: // Write this block On 2014/07/28 17:51:37, robertphillips wrote: > The 'k' prefix is usually reserved for comments. I think template parameters > usually just get a leading capital. Done.
The CQ bit was checked by krajcevski@google.com
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/krajcevski@google.com/421593004/10001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: Build-Win-VS2013-x86-Debug-Trybot on tryserver.skia (http://108.170.220.76:10117/builders/Build-Win-VS2013-x86-Debug-Trybot/builds...)
The CQ bit was checked by krajcevski@google.com
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/krajcevski@google.com/421593004/30001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: Build-Win-VS2013-x86-Debug-Trybot on tryserver.skia (http://108.170.220.76:10117/builders/Build-Win-VS2013-x86-Debug-Trybot/builds...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Build-Win-VS2013-x86-Debug-Trybot on tryserver.skia (http://108.170.220.76:10117/builders/Build-Win-VS2013-x86-Debug-Trybot/builds...)
The CQ bit was checked by krajcevski@google.com
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/krajcevski@google.com/421593004/30001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: Build-Win-VS2013-x86-Debug-Trybot on tryserver.skia (http://108.170.220.76:10117/builders/Build-Win-VS2013-x86-Debug-Trybot/builds...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Build-Win-VS2013-x86-Debug-Trybot on tryserver.skia (http://108.170.220.76:10117/builders/Build-Win-VS2013-x86-Debug-Trybot/builds...)
The CQ bit was checked by krajcevski@google.com
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/krajcevski@google.com/421593004/30001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: Build-Win-VS2013-x86-Debug-Trybot on tryserver.skia (http://108.170.220.76:10117/builders/Build-Win-VS2013-x86-Debug-Trybot/builds...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Build-Win-VS2013-x86-Debug-Trybot on tryserver.skia (http://108.170.220.76:10117/builders/Build-Win-VS2013-x86-Debug-Trybot/builds...)
The CQ bit was checked by krajcevski@google.com
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/krajcevski@google.com/421593004/30001
Message was sent while issue was closed.
Change committed as d5e46c7893afdd5976c1581a2ae81168252f5dec |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
