While removing compile warnings r128163 changed the logic of code for entering accelerated compositing mode, resulting in never being able to enter it.
Created attachment 163800 [details] Patch
Comment on attachment 163800 [details] Patch Fixes regression introduced by r128163 while removing compile warnings as intended in r128163.
Comment on attachment 163800 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=163800&action=review > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:501 > + EINA_SAFETY_ON_FALSE_RETURN_VAL(!priv->evasGl, false); Is below code is more readable instead of above one? EINA_SAFETY_ON_TRUE_RETURN_VAL(priv->evasGl, false);
(In reply to comment #3) > (From update of attachment 163800 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=163800&action=review > > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:501 > > + EINA_SAFETY_ON_FALSE_RETURN_VAL(!priv->evasGl, false); > Is below code is more readable instead of above one? > EINA_SAFETY_ON_TRUE_RETURN_VAL(priv->evasGl, false); Using EINA_SAFETY_ON_TRUE_RETURN_VAL(priv->evasGl, false); doesn't compile. If we want to use EINA_SAFETY_ON_TRUE_RETURN_VAL(), we have to put !! in front of priv->evasGl. Which one should I use? EINA_SAFETY_ON_FALSE_RETURN_VAL(!priv->evasGl, false); or EINA_SAFETY_ON_TRUE_RETURN_VAL(!!priv->evasGl, false);
(In reply to comment #4) > Using EINA_SAFETY_ON_TRUE_RETURN_VAL(priv->evasGl, false); doesn't compile. > If we want to use EINA_SAFETY_ON_TRUE_RETURN_VAL(), we have to put !! in front of priv->evasGl. > Which one should I use? > EINA_SAFETY_ON_FALSE_RETURN_VAL(!priv->evasGl, false); or > EINA_SAFETY_ON_TRUE_RETURN_VAL(!!priv->evasGl, false); Something is strange. I succeed to compile with below code in my linux box. EINA_SAFETY_ON_TRUE_RETURN_VAL(priv->evasGl, false); Could you check this again ?
(In reply to comment #5) > (In reply to comment #4) > > Using EINA_SAFETY_ON_TRUE_RETURN_VAL(priv->evasGl, false); doesn't compile. > > If we want to use EINA_SAFETY_ON_TRUE_RETURN_VAL(), we have to put !! in front of priv->evasGl. > > Which one should I use? > > EINA_SAFETY_ON_FALSE_RETURN_VAL(!priv->evasGl, false); or > > EINA_SAFETY_ON_TRUE_RETURN_VAL(!!priv->evasGl, false); > Something is strange. I succeed to compile with below code in my linux box. > EINA_SAFETY_ON_TRUE_RETURN_VAL(priv->evasGl, false); > Could you check this again ? You have to build with --tiled-backing-store option to include that code :)
Comment on attachment 163800 [details] Patch Frankly, EINA_SAFETY_ON_NULL_RETURN_VAL is correct I think. But, it was changed due to compilation warning. So, if you need to check if priv->evasGl is null, why don't you use *if* condition explicitly ? if (priv->evasGl) { EINA_LOG_DOM_WARN(_ewk_log_dom, "bla bla"); return false; }
Created attachment 164479 [details] Patch
(In reply to comment #7) > (From update of attachment 163800 [details]) > Frankly, EINA_SAFETY_ON_NULL_RETURN_VAL is correct I think. But, it was changed due to compilation warning. So, if you need to check if priv->evasGl is null, why don't you use *if* condition explicitly ? > if (priv->evasGl) { > EINA_LOG_DOM_WARN(_ewk_log_dom, "bla bla"); > return false; > } I agree that EINA_SAFETY_ON_NULL_RETURN_VAL is correct. Changed the code as you suggested.
Comment on attachment 164479 [details] Patch Clearing flags on attachment: 164479 Committed r128844: <http://trac.webkit.org/changeset/128844>
All reviewed patches have been landed. Closing bug.
Comment on attachment 164479 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164479&action=review > Source/WebKit2/ChangeLog:3 > + [EFL][WK2] Regression (r128163) Please add a title after Regression (r128163): > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:-501 > - EINA_SAFETY_ON_NULL_RETURN_VAL(priv->evasGl, false); EINA_SAFETY_ON_TRUE_RETURN_VAL(priv->evasGl, false); ?
(In reply to comment #12) > (From update of attachment 164479 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=164479&action=review > > > Source/WebKit2/ChangeLog:3 > > + [EFL][WK2] Regression (r128163) > > Please add a title after Regression (r128163): > > > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:-501 > > - EINA_SAFETY_ON_NULL_RETURN_VAL(priv->evasGl, false); > > EINA_SAFETY_ON_TRUE_RETURN_VAL(priv->evasGl, false); ? As previous comments mentioned, EINA_SAFETY_ON_TRUE_RETURN_VAL(priv->evasGl, false) makes a below build break with --tiled-backing-store option. /home/gyuyoung/webkit/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:501:5: error: invalid conversion from ‘Evas_GL* {aka _Evas_GL*}’ to ‘long int’ [-fpermissive]
(In reply to comment #13) > (In reply to comment #12) > > (From update of attachment 164479 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=164479&action=review > > > > > Source/WebKit2/ChangeLog:3 > > > + [EFL][WK2] Regression (r128163) > > > > Please add a title after Regression (r128163): > > > > > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:-501 > > > - EINA_SAFETY_ON_NULL_RETURN_VAL(priv->evasGl, false); > > > > EINA_SAFETY_ON_TRUE_RETURN_VAL(priv->evasGl, false); ? > > As previous comments mentioned, EINA_SAFETY_ON_TRUE_RETURN_VAL(priv->evasGl, false) makes a below build break with --tiled-backing-store option. > > /home/gyuyoung/webkit/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:501:5: error: invalid conversion from ‘Evas_GL* {aka _Evas_GL*}’ to ‘long int’ [-fpermissive] Maybe EINA_SAFETY_ON_TRUE_RETURN_VAL(!!priv->evasGl, false); would have worked then?
(In reply to comment #14) /home/gyuyoung/webkit/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:501:5: error: invalid conversion from ‘Evas_GL* {aka _Evas_GL*}’ to ‘long int’ [-fpermissive] > > Maybe EINA_SAFETY_ON_TRUE_RETURN_VAL(!!priv->evasGl, false); would have worked then? As I said comment #7, I think EINA_SAFETY_ON_NULL_RETURN_VAL is more proper because this is for checking if priv->evasGL is not null. Generally, it looks EINA_SAFETY_ON_TRUE | FALSE_RETURN_VAL() are used for checking specific condition rather than checking if null. So, I thought that explicit *if* condition is better than EINA_SAFETY_ON_TRUE | FALSE_RETURN_VAL for null checking.