Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2416)
Issues Repositories Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

Issue 6301044: fix warnings on Mac in samplecode (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years ago by caryclark1
Modified:
12 years ago
Reviewers:
epoger, bsalomon
CC:
skia-review_googlegroups.com
Base URL:
http://skia.googlecode.com/svn/trunk/
Visibility:
Public.

Description

fix warnings on Mac in ssamplecode Fix these class of warnings: - unused functions - unused locals - sign mismatch - missing function prototypes - missing newline at end of file - 64 to 32 bit truncation The changes prefer to link in dead code in the debug build with 'if (false)' than to comment it out, but trivial cases are commented out or sometimes deleted if it appears to be a copy/paste error. Committed: https://code.google.com/p/skia/source/detail?r=4183

Patch Set 1 #

Total comments: 20

Patch Set 2 : #

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+70 lines, -64 lines) Patch
M samplecode/OverView.cpp View 1 chunk +4 lines, -0 lines 0 comments Download
M samplecode/SampleAARectModes.cpp View 1 chunk +0 lines, -12 lines 0 comments Download
M samplecode/SampleAll.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M samplecode/SampleApp.cpp View 1 9 chunks +18 lines, -2 lines 0 comments Download
M samplecode/SampleArc.cpp View 1 chunk +1 line, -1 line 0 comments Download
M samplecode/SampleBlur.cpp View 1 1 chunk +3 lines, -0 lines 0 comments Download
M samplecode/SampleCircle.cpp View 1 1 chunk +3 lines, -0 lines 0 comments Download
M samplecode/SampleClip.cpp View 1 chunk +0 lines, -4 lines 0 comments Download
M samplecode/SampleColorFilter.cpp View 1 1 chunk +3 lines, -1 line 0 comments Download
M samplecode/SampleGradients.cpp View 1 1 chunk +3 lines, -1 line 0 comments Download
M samplecode/SampleHairModes.cpp View 1 chunk +0 lines, -12 lines 0 comments Download
M samplecode/SampleLineClipper.cpp View 1 1 chunk +3 lines, -1 line 0 comments Download
M samplecode/SampleOverflow.cpp View 1 2 2 chunks +3 lines, -1 line 0 comments Download
M samplecode/SamplePatch.cpp View 1 chunk +0 lines, -11 lines 0 comments Download
M samplecode/SamplePathClip.cpp View 1 chunk +1 line, -1 line 0 comments Download
M samplecode/SampleRegion.cpp View 1 1 chunk +8 lines, -2 lines 0 comments Download
M samplecode/SampleShaderText.cpp View 1 chunk +1 line, -1 line 0 comments Download
M samplecode/SampleSlides.cpp View 3 chunks +3 lines, -3 lines 0 comments Download
M samplecode/SampleStrokeText.cpp View 1 1 chunk +3 lines, -1 line 0 comments Download
M samplecode/SampleTextAlpha.cpp View 1 2 chunks +5 lines, -2 lines 0 comments Download
M samplecode/SampleXfermodesBlur.cpp View 2 chunks +1 line, -4 lines 0 comments Download
M samplecode/TransitionView.cpp View 2 chunks +5 lines, -2 lines 0 comments Download

Messages

Total messages: 4
bsalomon
http://codereview.appspot.com/6301044/diff/1/samplecode/SampleApp.cpp File samplecode/SampleApp.cpp (right): http://codereview.appspot.com/6301044/diff/1/samplecode/SampleApp.cpp#newcode602 samplecode/SampleApp.cpp:602: GrContext* GetGr(); wow http://codereview.appspot.com/6301044/diff/1/samplecode/SampleOverflow.cpp File samplecode/SampleOverflow.cpp (right): http://codereview.appspot.com/6301044/diff/1/samplecode/SampleOverflow.cpp#newcode59 samplecode/SampleOverflow.cpp:59: ...
12 years ago (2012-06-05 19:16:58 UTC) #1
bsalomon
On 2012/06/05 19:16:58, bsalomon wrote: > http://codereview.appspot.com/6301044/diff/1/samplecode/SampleApp.cpp > File samplecode/SampleApp.cpp (right): > > http://codereview.appspot.com/6301044/diff/1/samplecode/SampleApp.cpp#newcode602 > ...
12 years ago (2012-06-05 19:19:57 UTC) #2
epoger
http://codereview.appspot.com/6301044/diff/1/samplecode/SampleBlur.cpp File samplecode/SampleBlur.cpp (right): http://codereview.appspot.com/6301044/diff/1/samplecode/SampleBlur.cpp#newcode56 samplecode/SampleBlur.cpp:56: fBM = make_bitmap(); please add comment saying why or, ...
12 years ago (2012-06-05 19:47:02 UTC) #3
caryclark1
12 years ago (2012-06-05 20:29:42 UTC) #4 
http://codereview.appspot.com/6301044/diff/1/samplecode/SampleApp.cpp
File samplecode/SampleApp.cpp (right):

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleApp.cpp#newcode602
samplecode/SampleApp.cpp:602: GrContext* GetGr();
Added comment (FIXME) to document moving this to a header.
On 2012/06/05 19:16:58, bsalomon wrote:
> wow

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleBlur.cpp
File samplecode/SampleBlur.cpp (right):

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleBlur.cpp#newcode56
samplecode/SampleBlur.cpp:56: fBM = make_bitmap();
On 2012/06/05 19:47:02, epoger wrote:
> please add comment saying why
> 
> or, alternatively...
> 
> #ifdef COMPILE_UNUSED_CODE_TO_KEEP_IT_FROM_ROTTING
>    blah;
> #endif
> 
> 
> That would serve two purposes: indicating why, and allowing us to easily
toggle
> for whatever reason.

Done.

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleCircle.cpp
File samplecode/SampleCircle.cpp (right):

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleCircle.cpp#newc...
samplecode/SampleCircle.cpp:61: test_circlebounds(canvas);
On 2012/06/05 19:47:02, epoger wrote:
> please add comment saying why

Done.

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleColorFilter.cpp
File samplecode/SampleColorFilter.cpp (right):

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleColorFilter.cpp...
samplecode/SampleColorFilter.cpp:136: if (false) {
On 2012/06/05 19:47:02, epoger wrote:
> please add comment saying why

Done.

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleGradients.cpp
File samplecode/SampleGradients.cpp (right):

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleGradients.cpp#n...
samplecode/SampleGradients.cpp:171: if (false) {
On 2012/06/05 19:47:02, epoger wrote:
> please add comment saying why

Done.

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleLineClipper.cpp
File samplecode/SampleLineClipper.cpp (right):

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleLineClipper.cpp...
samplecode/SampleLineClipper.cpp:212: if (false) {
On 2012/06/05 19:47:02, epoger wrote:
> please add comment saying why

Done.

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleOverflow.cpp
File samplecode/SampleOverflow.cpp (right):

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleOverflow.cpp#ne...
samplecode/SampleOverflow.cpp:59: #ifdef SK_SCALAR_IS_FLOATx
Added comment to document your question. I don't know either.
On 2012/06/05 19:16:58, bsalomon wrote:
> I see we do this in other places but I don't get it. Is there actually a macro
> with this name or is effectively "#if 0" but documenting that the code only
> works for scalar=float?

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleRegion.cpp
File samplecode/SampleRegion.cpp (right):

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleRegion.cpp#newc...
samplecode/SampleRegion.cpp:326: if (false) {
On 2012/06/05 19:47:02, epoger wrote:
> please add comment saying why

Done.

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleStrokeText.cpp
File samplecode/SampleStrokeText.cpp (right):

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleStrokeText.cpp#...
samplecode/SampleStrokeText.cpp:135: lettersToBitmap2(&bm, "Test Case", paint,
SkBitmap::kARGB_4444_Config);
On 2012/06/05 19:47:02, epoger wrote:
> please add comment saying why

Done.

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleTextAlpha.cpp
File samplecode/SampleTextAlpha.cpp (right):

http://codereview.appspot.com/6301044/diff/1/samplecode/SampleTextAlpha.cpp#n...
samplecode/SampleTextAlpha.cpp:86: if (false) {
On 2012/06/05 19:47:02, epoger wrote:
> please add comment saying why

Done.
Sign in to reply to this message.

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