Bug 83562

Summary: [CMake] CMake SVG Code Generation fails to generate code for extra defines
Product: WebKit Reporter: Dominik Röttsches (drott) <d-r>
Component: WebCore Misc.Assignee: Dominik Röttsches (drott) <d-r>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, gyuyoung.kim, morrita, paroga, rakuco, rwlbuis, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 83564    
Attachments:
Description Flags
Fixing CMake build wrt SVG code generation
none
Fixing CMake build wrt SVG code generation
none
Fixing CMake build wrt SVG code generation none

Description Dominik Röttsches (drott) 2012-04-10 04:20:00 PDT
Mapping e.g. from CMAKE ON/OFF to ENABLE_FILTERS=ON is incorrect since the string value ON is not mapped to enabling code inside
#if ENABLE_FILTERS
for example.
See svgtags.in.

We need to generate ENABLE_FILTERS=1 for this to work, compare:
http://trac.webkit.org/browser/trunk/Source/WebCore/DerivedSources.make#L858

This should unskip a couple of SVG related tests on EFL.
Comment 1 Dominik Röttsches (drott) 2012-04-10 04:20:52 PDT
Also, meant to say that this should fix a couple of currently failing SVG tests that rely on SVG filters.
Comment 2 Dominik Röttsches (drott) 2012-04-10 05:17:23 PDT
Created attachment 136431 [details]
Fixing CMake build wrt SVG code generation

This should bring EFL test failures down by at least 17 cases.
Comment 3 Rob Buis 2012-04-10 05:25:07 PDT
Comment on attachment 136431 [details]
Fixing CMake build wrt SVG code generation

View in context: https://bugs.webkit.org/attachment.cgi?id=136431&action=review

I agree with this patch. If you can reupload the patch with typo fix I can r+.

> Source/WebCore/CMakeLists.txt:2584
> +# SVG extra defines need to map to a numerical value for correct preprocessing of svtags.in.

svgtags.in

> Source/WebCore/CMakeLists.txt:2590
> +    LIST(APPEND SVG_EXTRA_DEFINES "ENABLE_SVG_FONTS=1")

Does this add a space separator between both defines?
Comment 4 Gyuyoung Kim 2012-04-10 05:26:26 PDT
CC'ing patrick.
Comment 5 Patrick R. Gansterer 2012-04-10 05:27:49 PDT
LGTM, since I have no better idea ATM
Comment 6 Dominik Röttsches (drott) 2012-04-10 05:34:52 PDT
Created attachment 136434 [details]
Fixing CMake build wrt SVG code generation

Thanks for the quick review, typo fixed.
Comment 7 Dominik Röttsches (drott) 2012-04-10 05:35:29 PDT
(In reply to comment #3)
> (From update of attachment 136431 [details])
> 
> > Source/WebCore/CMakeLists.txt:2590
> > +    LIST(APPEND SVG_EXTRA_DEFINES "ENABLE_SVG_FONTS=1")
> 
> Does this add a space separator between both defines?

Yes it does.
Comment 8 Patrick R. Gansterer 2012-04-10 05:37:55 PDT
Comment on attachment 136434 [details]
Fixing CMake build wrt SVG code generation

View in context: https://bugs.webkit.org/attachment.cgi?id=136434&action=review

> Source/WebCore/CMakeLists.txt:2585
> +SET(SVG_EXTRA_DEFINES, "")

missed a point: comma after SVG_EXTRA_DEFINES
Comment 9 Dominik Röttsches (drott) 2012-04-10 05:42:35 PDT
Created attachment 136436 [details]
Fixing CMake build wrt SVG code generation

(In reply to comment #8)
> (From update of attachment 136434 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=136434&action=review
> 
> > Source/WebCore/CMakeLists.txt:2585
> > +SET(SVG_EXTRA_DEFINES, "")
> 
> missed a point: comma after SVG_EXTRA_DEFINES

Thanks. Fixed.
Comment 10 Rob Buis 2012-04-10 07:22:17 PDT
Comment on attachment 136436 [details]
Fixing CMake build wrt SVG code generation

Looks fine. Should I cq+ too?
Comment 11 Dominik Röttsches (drott) 2012-04-10 07:24:33 PDT
(In reply to comment #10)
> (From update of attachment 136436 [details])
> Looks fine. Should I cq+ too?

Yes please.
Comment 12 Rob Buis 2012-04-10 07:25:27 PDT
Comment on attachment 136436 [details]
Fixing CMake build wrt SVG code generation

Looks fine. Should I cq+ too?
Comment 13 Dominik Röttsches (drott) 2012-04-10 07:26:37 PDT
Thanks, Rob & Paroga!
Comment 14 WebKit Review Bot 2012-04-10 14:05:43 PDT
Comment on attachment 136436 [details]
Fixing CMake build wrt SVG code generation

Clearing flags on attachment: 136436

Committed r113763: <http://trac.webkit.org/changeset/113763>
Comment 15 WebKit Review Bot 2012-04-10 14:05:49 PDT
All reviewed patches have been landed.  Closing bug.