Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
108343006:
create simple 'skpmaker' tool to generate SKP files for testing (Closed)
|
Created:
7 years ago by epoger Modified:
6 years, 11 months ago CC:
skia-review_googlegroups.com Base URL:
https://skia.googlesource.com/skia.git@master Visibility:
Public. |
Descriptioncreate simple 'skpmaker' tool to generate SKP files for testing
(needed to create self-test for render_pictures)
BUG=skia:1943
Committed: http://code.google.com/p/skia/source/detail?r=12864
Patch Set 1 #
Total comments: 6
Patch Set 2 : rebase #Patch Set 3 : happy new year #
Messages
Total messages: 11 (0 generated)
Ready for review at patchset 1. I generated a few SKPs using the tool, and converted them into PNGs using render_pictures... they looked as expected. https://codereview.chromium.org/108343006/diff/1/gyp/tools.gyp File gyp/tools.gyp (right): https://codereview.chromium.org/108343006/diff/1/gyp/tools.gyp#newcode27 gyp/tools.gyp:27: 'skpmaker', rearranged these into alphabetical order, and added 'skpmaker' to the list
Ravi, can you review this please? Leon is out until Jan 7.
https://codereview.chromium.org/108343006/diff/1/tools/skpmaker.cpp File tools/skpmaker.cpp (right): https://codereview.chromium.org/108343006/diff/1/tools/skpmaker.cpp#newcode2 tools/skpmaker.cpp:2: * Copyright 2013 Google Inc. 2014 :) https://codereview.chromium.org/108343006/diff/1/tools/skpmaker.cpp#newcode75 tools/skpmaker.cpp:75: #if !defined SK_BUILD_FOR_IOS What does this section mean? Define main only if it is not an iOS build?
Please see patchset 3... https://codereview.chromium.org/108343006/diff/1/tools/skpmaker.cpp File tools/skpmaker.cpp (right): https://codereview.chromium.org/108343006/diff/1/tools/skpmaker.cpp#newcode2 tools/skpmaker.cpp:2: * Copyright 2013 Google Inc. On 2014/01/02 16:29:35, rmistry wrote: > 2014 :) Thanks, Ryan Seacrest! Fixed. https://codereview.chromium.org/108343006/diff/1/tools/skpmaker.cpp#newcode75 tools/skpmaker.cpp:75: #if !defined SK_BUILD_FOR_IOS On 2014/01/02 16:29:35, rmistry wrote: > What does this section mean? Define main only if it is not an iOS build? I suppose so. I just copied this from other code in the tools dir... epoger@wpgntat-ubiq141:~/src/skia/red/skia/tools$ grep --after-context=2 SK_BUILD_FOR_IOS *.cpp bbh_shootout.cpp:#if !defined(SK_BUILD_FOR_IOS) && !defined(SK_BUILD_FOR_NACL) bbh_shootout.cpp-int main(int argc, char** argv) { bbh_shootout.cpp- return tool_main(argc, argv); -- bench_pictures_main.cpp:#if !defined SK_BUILD_FOR_IOS bench_pictures_main.cpp-int main(int argc, char * const argv[]) { bench_pictures_main.cpp- return tool_main(argc, (char**) argv); -- filtermain.cpp:#if !defined SK_BUILD_FOR_IOS filtermain.cpp-int main(int argc, char * const argv[]) { filtermain.cpp- return tool_main(argc, (char**) argv); -- pinspect.cpp:#if !defined SK_BUILD_FOR_IOS pinspect.cpp-int main(int argc, char * const argv[]) { pinspect.cpp- return tool_main(argc, (char**) argv); -- render_pdfs_main.cpp:#if !defined SK_BUILD_FOR_IOS render_pdfs_main.cpp-int main(int argc, char * const argv[]) { render_pdfs_main.cpp- return tool_main(argc, (char**) argv); -- render_pictures_main.cpp:#if !defined SK_BUILD_FOR_IOS render_pictures_main.cpp-int main(int argc, char * const argv[]) { render_pictures_main.cpp- return tool_main(argc, (char**) argv); -- skdiff_image.cpp:#if !defined SK_BUILD_FOR_IOS skdiff_image.cpp-int main(int argc, char * const argv[]) { skdiff_image.cpp- return tool_main(argc, (char**) argv); -- skdiff_main.cpp:#if !defined SK_BUILD_FOR_IOS skdiff_main.cpp-int main(int argc, char * const argv[]) { skdiff_main.cpp- return tool_main(argc, (char**) argv); -- skhello.cpp:#if !defined SK_BUILD_FOR_IOS skhello.cpp-int main(int argc, char * const argv[]) { skhello.cpp- return tool_main(argc, (char**) argv); -- skimage_main.cpp:#if !defined SK_BUILD_FOR_IOS skimage_main.cpp-int main(int argc, char * const argv[]) { skimage_main.cpp- return tool_main(argc, (char**) argv); -- skpmaker.cpp:#if !defined SK_BUILD_FOR_IOS skpmaker.cpp-int main(int argc, char * const argv[]) { skpmaker.cpp- return tool_main(argc, (char**) argv); -- test_image_decoder.cpp:#if !defined SK_BUILD_FOR_IOS test_image_decoder.cpp-int main(int argc, char * const argv[]) { test_image_decoder.cpp- return tool_main(argc, (char**) argv);
https://codereview.chromium.org/108343006/diff/1/tools/skpmaker.cpp File tools/skpmaker.cpp (right): https://codereview.chromium.org/108343006/diff/1/tools/skpmaker.cpp#newcode75 tools/skpmaker.cpp:75: #if !defined SK_BUILD_FOR_IOS On 2014/01/02 16:40:30, epoger wrote: > On 2014/01/02 16:29:35, rmistry wrote: > > What does this section mean? Define main only if it is not an iOS build? > > I suppose so. I just copied this from other code in the tools dir... > > epoger@wpgntat-ubiq141:~/src/skia/red/skia/tools$ grep --after-context=2 > SK_BUILD_FOR_IOS *.cpp > bbh_shootout.cpp:#if !defined(SK_BUILD_FOR_IOS) && !defined(SK_BUILD_FOR_NACL) > bbh_shootout.cpp-int main(int argc, char** argv) { > bbh_shootout.cpp- return tool_main(argc, argv); > -- > bench_pictures_main.cpp:#if !defined SK_BUILD_FOR_IOS > bench_pictures_main.cpp-int main(int argc, char * const argv[]) { > bench_pictures_main.cpp- return tool_main(argc, (char**) argv); > -- > filtermain.cpp:#if !defined SK_BUILD_FOR_IOS > filtermain.cpp-int main(int argc, char * const argv[]) { > filtermain.cpp- return tool_main(argc, (char**) argv); > -- > pinspect.cpp:#if !defined SK_BUILD_FOR_IOS > pinspect.cpp-int main(int argc, char * const argv[]) { > pinspect.cpp- return tool_main(argc, (char**) argv); > -- > render_pdfs_main.cpp:#if !defined SK_BUILD_FOR_IOS > render_pdfs_main.cpp-int main(int argc, char * const argv[]) { > render_pdfs_main.cpp- return tool_main(argc, (char**) argv); > -- > render_pictures_main.cpp:#if !defined SK_BUILD_FOR_IOS > render_pictures_main.cpp-int main(int argc, char * const argv[]) { > render_pictures_main.cpp- return tool_main(argc, (char**) argv); > -- > skdiff_image.cpp:#if !defined SK_BUILD_FOR_IOS > skdiff_image.cpp-int main(int argc, char * const argv[]) { > skdiff_image.cpp- return tool_main(argc, (char**) argv); > -- > skdiff_main.cpp:#if !defined SK_BUILD_FOR_IOS > skdiff_main.cpp-int main(int argc, char * const argv[]) { > skdiff_main.cpp- return tool_main(argc, (char**) argv); > -- > skhello.cpp:#if !defined SK_BUILD_FOR_IOS > skhello.cpp-int main(int argc, char * const argv[]) { > skhello.cpp- return tool_main(argc, (char**) argv); > -- > skimage_main.cpp:#if !defined SK_BUILD_FOR_IOS > skimage_main.cpp-int main(int argc, char * const argv[]) { > skimage_main.cpp- return tool_main(argc, (char**) argv); > -- > skpmaker.cpp:#if !defined SK_BUILD_FOR_IOS > skpmaker.cpp-int main(int argc, char * const argv[]) { > skpmaker.cpp- return tool_main(argc, (char**) argv); > -- > test_image_decoder.cpp:#if !defined SK_BUILD_FOR_IOS > test_image_decoder.cpp-int main(int argc, char * const argv[]) { > test_image_decoder.cpp- return tool_main(argc, (char**) argv); Yeah, this is just the weird way we trampoline into our apps on iOS. experimental/SimpleiOSApp/SimpleApp.mm has the glue if you're interested.
LGTM
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/epoger@google.com/108343006/90001
Message was sent while issue was closed.
Change committed as 12864
Message was sent while issue was closed.
A revert of this CL has been created in https://codereview.chromium.org/102243012/ by epoger@google.com. The reason for reverting is: broke some builds.
Message was sent while issue was closed.
On 2014/01/02 18:51:26, epoger wrote: > A revert of this CL has been created in > https://codereview.chromium.org/102243012/ by mailto:epoger@google.com. > > The reason for reverting is: broke some builds. The CL lgtm. Do you plan to resubmit once all builds are fixed?
Message was sent while issue was closed.
On 2014/01/07 18:56:10, scroggo wrote: > On 2014/01/02 18:51:26, epoger wrote: > > A revert of this CL has been created in > > https://codereview.chromium.org/102243012/ by mailto:epoger@google.com. > > > > The reason for reverting is: broke some builds. > > The CL lgtm. Do you plan to resubmit once all builds are fixed? The CL was properly landed as https://code.google.com/p/skia/source/detail?r=12866 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
