Page MenuHomePhabricator

evas is broken
Closed, ResolvedPublic

Description

efl commit fb048e73120b broke drm *software* rendering, and I'm hearing reports that it also causes X11 to create a gl context when performing software rendering.

enlightenment crashes without rendering a single frame.

I hate dirty pages as much as the next guy, but I do love me some pixels.

ain't got no pixels today. :(

ManMower created this task.Jan 16 2018, 3:24 PM
zmike added a subscriber: zmike.Jan 16 2018, 3:39 PM

Every released version of Enlightenment, as well as some other apps, will be broken with this change but Enlightenment runs again so it's fine.

and I'm hearing reports that it also causes X11 to create a gl

WTH? It does the opposite. It disables any init/loading of OSMesa in the sw generic core UNTIL some gl stuff is needed/used. Before OSMesa was always loaded no matter what (thus all the dirty pages... and dirty pages are the equivalent of a malloc with all pages in it touched, so it's real). 8Mb per process is a hell of a lot of saving. anyway... but it's the exact opposite of what you describe above.

I didn't indeed test with wl_drm as my main machine can't use that. But the resolved commit seems to be in the right direction, but I think the issue is more that glapi is NULL. it shouldn't be. At least with sw x11 it worked right and osmesa worked with glapi being a valid ptr as it's evaluated WHEN called. something the wl engine does causes the glapi to become null and fail. the glapi just from the diff i assume is the evas_gl api... and this does work as software only rendering with "glviewsimple" in elm test worked. as did "glview many gears" both of which don't try and switch accel/engine and use default (thus i can know they use osemesa and software). at least the wl_shm client engine does the right thing, so i guess there is something SPECIFIC to the drm engine with regards to gl api handling. it should do what software_x11 and wl_shm do.

as an aside... why is e doing anything with the evas_gl api there? shouldn't that be entirely hidden inside evas (the unbind)? it's totally unneeded for the software drm renderer as no real gpu/gl device exists... ? i'm wondering...

ran this on an intel laptop with software drm.

elementary_test -to glviewsimple

works.... drm engine. so... first... why is this NULL? ok. scrap my previous question. this is specifically the gl init path... then, BUT my question changes. why is a gl init func being called for a non-gl engine setup? why? there is no value or reason to get a glapi from evas and do this binding/unbinding if we use software. if its an osemsa glapi context.. then this should always have been NULL as osemesa wouldn't support the wayland bindings. the only init to UnbindWaylandDisplay is actually in gl_common (evas_gl_api_ext.c) ... so my questions above stand. why is this code being executed for a SAOFTWARE engine as you claim as it's clearly a gl init path, and then why would you expect a SOFTWARE osmesa renderer to support the wayland unbind/bind etc. ... this SHOULD be null

ok ... one point. we could argue that we could set function ptrs instead of to null to some "nop func"... then you would just have silent "ignore this" results. that's another question, but first to my questions above... i think that evas is doing the right thing. especially now. that func ptr should be NULL.

jpeg added a subscriber: jpeg.Jan 16 2018, 9:46 PM
jpeg reopened this task as Open.Jan 16 2018, 10:15 PM

The patches submitted show that application logic must be adapted, which is not acceptable. I'll submit something soon.

well my reading of the patch tells me that the application was broken to begin with... see my questions above.

jpeg added a comment.Jan 17 2018, 2:00 AM

It was assuming that if evas_gl_new() returns something then surely evas gl must work. The null checks can remain, but the assumption was absolutely not stupid.

but why was this code even running for a software compositor? oddly enough evas_gl_new seems to work for the elm gl tests (glviewsimple for example). it works with drm+software.

oh wait... let me guess. i have osemesa installed... and @ManMower does not? thus it works for me? should be even return a gl api if there is no ability to render gl (osmesa couldn't be loaded)?

Yeah, don't have OSMesa installed. It's not a build dep, so I didn't bother.

aaaaah ok. with osmesa NOT installed... do you expect the evas_gl api to be NULL or not? because gl rendering can't work without a gl library backing it... ?

and still the question remains... what has that patch got to do with fixing software rendering? it's in the gl init code... not sw. ?

So my question still stands. the patch has nothing to do with software. the fix./change to code above is in the gl init path .. or SHOULD be. someone please explain if i'm wrong...

and without osmesa installed you get a NULL egl api handle... this is actually how it SHOULD work. gl is not possible. so ... what about these changes i made actually breaks anything that shouldn't be? were we returning a non-null gl api handle even if osmesa was not found... but then how could that work anyway? i am pretty certain long ago we were returning a null gl api handle when no osmesa was found and perhaps someone changed that to make it always return a handle anyway?

my take on this is that my changes inadvertently fixed the above behavior to be back to where it was before and should be. if you think that's wrong... i'd like to know why. :)

jpeg added a comment.Jan 22 2018, 3:59 AM

and without osmesa installed you get a NULL egl api handle... this is actually how it SHOULD work. gl is not possible. so ... what about these changes i made actually breaks anything that shouldn't be? were we returning a non-null gl api handle even if osmesa was not found... but then how could that work anyway? i am pretty certain long ago we were returning a null gl api handle when no osmesa was found and perhaps someone changed that to make it always return a handle anyway?

Between fb048e73120b3909 and 24447641d38cb9bf7b evas_gl_new() returned non-NULL even if OSMesa is not present. All actual function calls would fail (context create, etc). Before and after those patches evas_gl_new() always returns NULL.

sure. but @zmike's patch above adds checks for the evas gl api handle that were not there before... that implies that it always returned an evas_gl api handle. but it's also in the gl init path which i was saying made no sense as @ManMower says it's the software path that fails to start... so i'm wondering first where the breakages are and are there any more issues to resolve? i'm not sure there are?

zmike added a comment.Jan 22 2018, 2:43 PM

Nope, it's all fixed at this point.