Bug 101392 - [EFL] Simplify the build system
Summary: [EFL] Simplify the build system
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-06 14:07 PST by Laszlo Gombos
Modified: 2012-11-06 16:59 PST (History)
8 users (show)

See Also:


Attachments
1st try (2.87 KB, patch)
2012-11-06 14:23 PST, Laszlo Gombos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Laszlo Gombos 2012-11-06 14:07:57 PST
Remove unused cmake variables, refactor build flags.
Comment 1 Laszlo Gombos 2012-11-06 14:23:47 PST
Created attachment 172648 [details]
1st try
Comment 2 Raphael Kubo da Costa (:rakuco) 2012-11-06 14:41:51 PST
Comment on attachment 172648 [details]
1st try

Looks good, I guess. These things are not enabled by default so they are not really tested by the EWS, but if you guys who are working on it say it's OK it's probably fine :-)
Comment 3 Yael 2012-11-06 15:19:58 PST
Comment on attachment 172648 [details]
1st try

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

> Source/cmake/OptionsEfl.cmake:178
> -  SET(WTF_USE_ACCELERATED_COMPOSITING 1)
>    ADD_DEFINITIONS(-DWTF_USE_ACCELERATED_COMPOSITING=1)

Is this still needed after WTF_USE_ACCELERATED_COMPOSITING was moved to platform.h ?

> Source/cmake/OptionsEfl.cmake:180
> -  SET(WTF_USE_COORDINATED_GRAPHICS 1)
>    ADD_DEFINITIONS(-DWTF_USE_COORDINATED_GRAPHICS=1)

ditto
Comment 4 WebKit Review Bot 2012-11-06 15:25:46 PST
Comment on attachment 172648 [details]
1st try

Clearing flags on attachment: 172648

Committed r133678: <http://trac.webkit.org/changeset/133678>
Comment 5 WebKit Review Bot 2012-11-06 15:25:50 PST
All reviewed patches have been landed.  Closing bug.
Comment 6 Laszlo Gombos 2012-11-06 16:29:53 PST
(In reply to comment #3)
> (From update of attachment 172648 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=172648&action=review
> 
> > Source/cmake/OptionsEfl.cmake:178
> > -  SET(WTF_USE_ACCELERATED_COMPOSITING 1)
> >    ADD_DEFINITIONS(-DWTF_USE_ACCELERATED_COMPOSITING=1)
> 
> Is this still needed after WTF_USE_ACCELERATED_COMPOSITING was moved to platform.h ?

At the moment WTF_USE_ACCELERATED_COMPOSITING is only enabled in OptionsEfl.cmake for EFL. 

I agree in that once we enable WTF_USE_ACCELERATED_COMPOSITING by default for EFL, it is best to do it in Platform.h and not in OptionsEfl.cmake. 

 /* Accelerated compositing */
-#if PLATFORM(MAC) || PLATFORM(IOS) || PLATFORM(QT) || (PLATFORM(WIN) && !OS(WINCE) && !PLATFORM(WIN_CAIRO))
+#if PLATFORM(MAC) || PLATFORM(IOS) || PLATFORM(QT) || PLATFORM(EFL) || (PLATFORM(WIN) && !OS(WINCE) && !PLATFORM(WIN_CAIRO))

Until this change is not made, we still need the above line in OptionsEfl.cmake.
Comment 7 Yael 2012-11-06 16:59:27 PST
(In reply to comment #6)
> (In reply to comment #3)
> > (From update of attachment 172648 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=172648&action=review
> > 
> > > Source/cmake/OptionsEfl.cmake:178
> > > -  SET(WTF_USE_ACCELERATED_COMPOSITING 1)
> > >    ADD_DEFINITIONS(-DWTF_USE_ACCELERATED_COMPOSITING=1)
> > 
> > Is this still needed after WTF_USE_ACCELERATED_COMPOSITING was moved to platform.h ?
> 
> At the moment WTF_USE_ACCELERATED_COMPOSITING is only enabled in OptionsEfl.cmake for EFL. 
> 
> I agree in that once we enable WTF_USE_ACCELERATED_COMPOSITING by default for EFL, it is best to do it in Platform.h and not in OptionsEfl.cmake. 
> 
>  /* Accelerated compositing */
> -#if PLATFORM(MAC) || PLATFORM(IOS) || PLATFORM(QT) || (PLATFORM(WIN) && !OS(WINCE) && !PLATFORM(WIN_CAIRO))
> +#if PLATFORM(MAC) || PLATFORM(IOS) || PLATFORM(QT) || PLATFORM(EFL) || (PLATFORM(WIN) && !OS(WINCE) && !PLATFORM(WIN_CAIRO))
> 
> Until this change is not made, we still need the above line in OptionsEfl.cmake.

My bad. I thought your other patch changed that .