Bug 65192 – [EFL] Do not crash if the Cairo surface cannot be created.

• Component: New Bugs
• Status: RESOLVED FIXED

[Raphael Kubo da Costa (:rakuco)]

[EFL] Do not crash if the Cairo surface cannot be created.

[Raphael Kubo da Costa (:rakuco)]

Created attachment 102028 [details]
Patch

[Raphael Kubo da Costa (:rakuco)]

CC'ing mrobinson instead of tkent, as this is cairo-related.

[Gyuyoung Kim]

Comment on attachment 102028 [details]
Patch

Attachment 102028 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9251384

[Martin Robinson]

Wouldn't it be better to use the current clip to figure out how big the surface should be rather than to just fail?

[Raphael Kubo da Costa (:rakuco)]

Created attachment 102164 [details]
Patch

[Leandro Pereira]

Comment on attachment 102164 [details]
Patch

The 'return false'  substituting 'return true'  in themePartCacheEntrySurfaceCreate() shouldn't be there.

[Leandro Pereira]

Created attachment 102179 [details]
Patch

[Martin Robinson]

Comment on attachment 102179 [details]
Patch

By my comment I actually was asking whether or not you could simply cache only what's within the clipping region, versus allocating a cache area the size of the entire widget.

[Raphael Kubo da Costa (:rakuco)]

Sorry for leaving this one behind for so long.

I was talking to Leandro about your question/suggestion a few minutes ago. If we were to cache what's visible, wouldn't we need to keep track of viewport size changes and all that, making the code much bigger?

[Martin Robinson]

(In reply to comment #9)
> Sorry for leaving this one behind for so long.
> 
> I was talking to Leandro about your question/suggestion a few minutes ago. If we were to cache what's visible, wouldn't we need to keep track of viewport size changes and all that, making the code much bigger?

I don't know exactly how your theme cache works, but it seems that if you should already be checking whether the widget changes size each time you draw. You'd just need to add one more condition determining whether or not the clip region has moved or changed size. I'm not sure how feasible that is.

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #10)
> I don't know exactly how your theme cache works, but it seems that if you should already be checking whether the widget changes size each time you draw. You'd just need to add one more condition determining whether or not the clip region has moved or changed size. I'm not sure how feasible that is.

After spending some time on this, it looks like implementing this suggestion could be feasible, but would require quite some effort. On the one hand, Edje (which actually renders the theme) has a hardcoded limit of 20000x20000 pixels for a widget's dimensions; on the other hand, the way RenderThemeEfl is currently implemented makes it a bit difficult to track the clipping region and decide where exactly in the cairo context to paint the rendered element (when scrolling, scaling and other things are taken into account it becomes even harder).

For now, I think it's OK to just not display elements which are too big (a 1-month-old GtkLauncher build I have here just displays the top of the textarea in the mentioned testcase, and my distro's Chromium displays mostly garbage after some point too).

I'm submitting a new patch with a better heuristic in a few minutes.

[Raphael Kubo da Costa (:rakuco)]

Created attachment 106118 [details]
Patch

[Raphael Kubo da Costa (:rakuco)]

Created attachment 106419 [details]
Be less strict about what sizes we consider to be invalid

[WebKit Review Bot]

Comment on attachment 106419 [details]
Be less strict about what sizes we consider to be invalid

Clearing flags on attachment: 106419

Committed r94578: <http://trac.webkit.org/changeset/94578>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.