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

Issue 6443112: Effects Bug Fix (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 4 months ago by chudy
Modified:
12 years, 4 months ago
Reviewers:
DerekS
CC:
skia-review_googlegroups.com
Base URL:
https://skia.googlecode.com/svn/trunk
Visibility:
Public.

Description

Effects Bug Fix Certain compilers in an effort to optimize code chop off files that are never used. By adding a flag to common conditions and variables we can force skia to recompile with global static initializers off. By making a call to SkGraphics::Init we now register all those functions that had been previously automatically excluded by the compiler. Committed: https://code.google.com/p/skia/source/detail?r=5057

Patch Set 1 #

Total comments: 1

Patch Set 2 : Make debugger runs make clean in order to recompile libraries with the correct flag #

Total comments: 2

Patch Set 3 : Addressed patch 2 comment #

Patch Set 4 : Switched to bash script instead of makefile for simplicity # #

Patch Set 5 : Script renamed #

Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -9 lines) Patch
M debugger/QT/SkDebuggerGUI.cpp View 1 2 2 chunks +6 lines, -2 lines 0 comments Download
A debugger/make_debugger.sh View 1 2 3 4 1 chunk +12 lines, -0 lines 0 comments Download
M gyp/common_conditions.gypi View 1 5 chunks +8 lines, -5 lines 0 comments Download
M gyp/common_variables.gypi View 1 2 chunks +3 lines, -1 line 0 comments Download
M gyp/debugger.gyp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 6
chudy
12 years, 4 months ago (2012-08-10 18:50:07 UTC) #1
DerekS
lgtm with one nit https://codereview.appspot.com/6443112/diff/1/gyp/common_variables.gypi File gyp/common_variables.gypi (right): https://codereview.appspot.com/6443112/diff/1/gyp/common_variables.gypi#newcode71 gyp/common_variables.gypi:71: 'skia_global_initializers%': 1, lets call it ...
12 years, 4 months ago (2012-08-10 18:57:54 UTC) #2
DerekS
https://codereview.appspot.com/6443112/diff/1006/debugger/QT/SkDebuggerGUI.cpp File debugger/QT/SkDebuggerGUI.cpp (right): https://codereview.appspot.com/6443112/diff/1006/debugger/QT/SkDebuggerGUI.cpp#newcode98 debugger/QT/SkDebuggerGUI.cpp:98: SkDebuggerGUI::~SkDebuggerGUI() {} for completeness we should have a matching ...
12 years, 4 months ago (2012-08-13 12:08:08 UTC) #3
chudy
https://codereview.appspot.com/6443112/diff/1006/debugger/QT/SkDebuggerGUI.cpp File debugger/QT/SkDebuggerGUI.cpp (right): https://codereview.appspot.com/6443112/diff/1006/debugger/QT/SkDebuggerGUI.cpp#newcode98 debugger/QT/SkDebuggerGUI.cpp:98: SkDebuggerGUI::~SkDebuggerGUI() {} On 2012/08/13 12:08:08, DerekS wrote: > for ...
12 years, 4 months ago (2012-08-13 13:04:06 UTC) #4
DerekS
lgtm, but call it make_debugger.sh
12 years, 4 months ago (2012-08-13 14:22:03 UTC) #5
chudy
12 years, 4 months ago (2012-08-13 14:27:58 UTC) #6
Committed in 5057. Thanks for the review

On Mon, Aug 13, 2012 at 10:22 AM,  <djsollen@google.com> wrote:
>
> lgtm, but call it make_debugger.sh
>
> https://codereview.appspot.com/6443112/



-- 
Karol Chudy
Phone: (203) 898-4233
chudy@google.com
Sign in to reply to this message.

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