106319 2013-01-08 03:52:37 -0800 [EFL] [WebGL] Remove any GLX dependencies from X11WindowResources. 2013-01-09 19:44:25 -0800 1 1 1 Unclassified WebKit WebKit EFL 528+ (Nightly build) PC Linux RESOLVED FIXED P2 Normal --- 105136 1 kalyan.kondapally kalyan.kondapally laszlo.gombos lucas.de.marchi noam ostap73 webkit.review.bot zeno oldest_to_newest 802195 0 kalyan.kondapally 2013-01-08 03:52:37 -0800 After https://bugs.webkit.org/show_bug.cgi?id=105136, we have GLX dependencies in X11WindowResources class. This is wrong and we should try to avoid any GL calls here( We could be using either GLX or EGL). we differ the setgeometry calls till we actually blit the texture contents to FBO in our platformLayer(GraphicsContext3DPrivate). We eventually call swapbuffers after the blit operation anyway. So, is the issue here that the buffers are not resized properly before the blit operation happens ?? (i.e calling glviewport with right attributes in GLPlatformSurface::setGeometry). 802275 1 ostap73 2013-01-08 08:44:23 -0800 (In reply to comment #0) > After https://bugs.webkit.org/show_bug.cgi?id=105136, we have GLX dependencies in X11WindowResources class. This is wrong and we should try to avoid any GL calls here( We could be using either GLX or EGL). > > we differ the setgeometry calls till we actually blit the texture contents to FBO in our platformLayer(GraphicsContext3DPrivate). We eventually call swapbuffers after the blit operation anyway. So, is the issue here that the buffers are not resized properly before the blit operation happens ?? (i.e calling glviewport with right attributes in GLPlatformSurface::setGeometry). As I wrote in 105136, glViewport doesn't help. 802288 2 kalyan.kondapally 2013-01-08 09:03:29 -0800 (In reply to comment #1) > (In reply to comment #0) > > After https://bugs.webkit.org/show_bug.cgi?id=105136, we have GLX dependencies in X11WindowResources class. This is wrong and we should try to avoid any GL calls here( We could be using either GLX or EGL). > As I wrote in 105136, glViewport doesn't help. K, I have a patch which does call swapbuffers in the platformLayer. This should handle all the cases i.e when surface is single buffered or double buffered and leave it to the surface to handle it as necessary. I haven't checked the issue with EGL but I assume the issue should be present(if the buffers are not being resized automatically). While using EGL, we are currently dependent on native window/pixmap (X11 in this case) to share content with UI process. 802918 3 181824 kalyan.kondapally 2013-01-08 19:35:23 -0800 Created attachment 181824 patch 803957 4 182047 kalyan.kondapally 2013-01-09 19:21:23 -0800 Created attachment 182047 patch v2 803959 5 182047 ostap73 2013-01-09 19:25:02 -0800 Comment on attachment 182047 patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=182047&action=review > Source/WebCore/platform/graphics/surfaces/glx/X11WindowResources.cpp:53 > + XFlush(m_sharedResources->x11Display()); For me XFlush didn't do anything good here. And glXSwapBuffers does it itself. At least mesa does (check mesa source at src/glx/glxcmds.c). 803964 6 kalyan.kondapally 2013-01-09 19:29:04 -0800 This patch handles only the GLX changes. EGL does seem to have the issue which needs to be handled separately. It would be good to have a layout test for this, to avoid breaking it. 803966 7 kalyan.kondapally 2013-01-09 19:33:09 -0800 (In reply to comment #5) > (From update of attachment 182047 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=182047&action=review > > > Source/WebCore/platform/graphics/surfaces/glx/X11WindowResources.cpp:53 > > + XFlush(m_sharedResources->x11Display()); > > For me XFlush didn't do anything good here. > And glXSwapBuffers does it itself. At least mesa does (check mesa source at src/glx/glxcmds.c). That is true. With EGL we would need this though or use EGLWAITCLIENT for example. This just ensures that the buffers on X are updated appropriately. This doesn't involve a round trip to XServer like XSync does, so shouldn't be that costly and doesn't add a overhead on glx side. 803978 8 182047 webkit.review.bot 2013-01-09 19:44:21 -0800 Comment on attachment 182047 patch v2 Clearing flags on attachment: 182047 Committed r139276: <http://trac.webkit.org/changeset/139276> 803979 9 webkit.review.bot 2013-01-09 19:44:25 -0800 All reviewed patches have been landed. Closing bug. 181824 2013-01-08 19:35:23 -0800 2013-01-09 19:21:23 -0800 patch glxchangesreview.patch text/plain 2729 kalyan.kondapally ZGlmZiAtLWdpdCBTb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCmluZGV4IDVhOGUzMTMuLmZiZmQ0MWMgMTAwNjQ0Ci0tLSBTb3VyY2UvV2ViQ29yZS9DaGFu Z2VMb2cKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDEz LTAxLTA4ICBLb25kYXBhbGx5IEthbHlhbiAgPGthbHlhbi5rb25kYXBhbGx5QGludGVsLmNvbT4K KworICAgICAgICBbRUZMXSBbV2ViR0xdIFJlbW92ZSBHTFggZGVwZW5kZW5jaWVzIGZyb20gWDEx V2luZG93UmVzb3VyY2VzLi4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTEwNjMxOQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgIFRoaXMgcGF0Y2ggcmVtb3ZlcyBHTFggc3BlY2lmaWMgY2FsbHMgZnJvbSBYMTFX aW5kb3dSZXNvdXJjZXMuCisgICAgICAgIFRoaXMgY2xhc3MgaXMgc2hhcmVkIGJ5IGJvdGggR0xY IGFuZCBFR0wgaW1wbGVtZW50YXRpb25zLgorCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3Mv c3VyZmFjZXMvZ2x4L0dMWFN1cmZhY2UuY3BwOgorICAgICAgICAoV2ViQ29yZTo6R0xYVHJhbnNw b3J0U3VyZmFjZTo6c2V0R2VvbWV0cnkpOgorICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL3N1 cmZhY2VzL2dseC9YMTFXaW5kb3dSZXNvdXJjZXMuY3BwOgorICAgICAgICAoV2ViQ29yZTo6WDEx T2ZmU2NyZWVuV2luZG93OjpyZVNpemVXaW5kb3cpOgorCiAyMDEzLTAxLTA4ICBKdXN0aW4gTm92 b3NhZCAgPGp1bm92QGdvb2dsZS5jb20+CiAKICAgICAgICAgQ2FudmFzUmVuZGVyaW5nQ29udGV4 dDJEOjpzZXRGb250IGFyZ3VtZW50IG1heSByZWZlcmVuY2UgZGVzdHJveWVkIG9iamVjdApAQCAt MjA5Niw3ICsyMTExLDcgQEAKIAogMjAxMy0wMS0wNiAgU2ViYXN0aWFuIERyw7ZnZSAgPHNlYmFz dGlhbi5kcm9lZ2VAY29sbGFib3JhLmNvLnVrPgogCi0gICAgICAgIEJ1Z8KgMTA2MTc3IC0gW0dT dHJlYW1lcl0gRG9uJ3QgY2FsbCBnc3RfdmlkZW9faW5mb19mcm9tX2NhcHMoKSB3aXRoIG5vbi1m aXhlZCBjYXBzCisgICAgICAgIEJ1ZyAxMDYxNzcgLSBbR1N0cmVhbWVyXSBEb24ndCBjYWxsIGdz dF92aWRlb19pbmZvX2Zyb21fY2FwcygpIHdpdGggbm9uLWZpeGVkIGNhcHMKICAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTEwNjE3NwogCiAgICAgICAgIFJl dmlld2VkIGJ5IE1hcnRpbiBSb2JpbnNvbi4KZGlmZiAtLWdpdCBTb3VyY2UvV2ViQ29yZS9wbGF0 Zm9ybS9ncmFwaGljcy9zdXJmYWNlcy9nbHgvR0xYU3VyZmFjZS5jcHAgU291cmNlL1dlYkNvcmUv cGxhdGZvcm0vZ3JhcGhpY3Mvc3VyZmFjZXMvZ2x4L0dMWFN1cmZhY2UuY3BwCmluZGV4IDBjNzc0 NmUuLmY1MDIzNTcgMTAwNjQ0Ci0tLSBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9z dXJmYWNlcy9nbHgvR0xYU3VyZmFjZS5jcHAKKysrIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dy YXBoaWNzL3N1cmZhY2VzL2dseC9HTFhTdXJmYWNlLmNwcApAQCAtNzUsNiArNzUsOCBAQCB2b2lk IEdMWFRyYW5zcG9ydFN1cmZhY2U6OnNldEdlb21ldHJ5KGNvbnN0IEludFJlY3QmIG5ld1JlY3Qp CiB7CiAgICAgR0xQbGF0Zm9ybVN1cmZhY2U6OnNldEdlb21ldHJ5KG5ld1JlY3QpOwogICAgIG1f bmF0aXZlUmVzb3VyY2UtPnJlU2l6ZVdpbmRvdyhuZXdSZWN0LCBtX2RyYXdhYmxlKTsKKyAgICAv LyBGb3JjZSByZXNpemUgb2YgR0wgc3VyZmFjZSBhZnRlciB3aW5kb3cgcmVzaXplLgorICAgIGds WFN3YXBCdWZmZXJzKHNoYXJlZERpc3BsYXkoKSwgbV9kcmF3YWJsZSk7CiB9CiAKIHZvaWQgR0xY VHJhbnNwb3J0U3VyZmFjZTo6c3dhcEJ1ZmZlcnMoKQpkaWZmIC0tZ2l0IFNvdXJjZS9XZWJDb3Jl L3BsYXRmb3JtL2dyYXBoaWNzL3N1cmZhY2VzL2dseC9YMTFXaW5kb3dSZXNvdXJjZXMuY3BwIFNv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3N1cmZhY2VzL2dseC9YMTFXaW5kb3dSZXNv dXJjZXMuY3BwCmluZGV4IDgwMDhhZmIuLjVhNTdhODEgMTAwNjQ0Ci0tLSBTb3VyY2UvV2ViQ29y ZS9wbGF0Zm9ybS9ncmFwaGljcy9zdXJmYWNlcy9nbHgvWDExV2luZG93UmVzb3VyY2VzLmNwcAor KysgU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvc3VyZmFjZXMvZ2x4L1gxMVdpbmRv d1Jlc291cmNlcy5jcHAKQEAgLTUwLDkgKzUwLDcgQEAgWDExT2ZmU2NyZWVuV2luZG93Ojp+WDEx T2ZmU2NyZWVuV2luZG93KCkKIHZvaWQgWDExT2ZmU2NyZWVuV2luZG93OjpyZVNpemVXaW5kb3co Y29uc3QgSW50UmVjdCYgbmV3UmVjdCwgY29uc3QgdWludDMyX3Qgd2luZG93SWQpCiB7CiAgICAg WFJlc2l6ZVdpbmRvdyhtX3NoYXJlZFJlc291cmNlcy0+eDExRGlzcGxheSgpLCB3aW5kb3dJZCwg bmV3UmVjdC53aWR0aCgpLCBuZXdSZWN0LmhlaWdodCgpKTsKLQotICAgIC8vIEZvcmNlIHJlc2l6 ZSBvZiBHTCBzdXJlZmFjZSBhZnRlciB3aW5kb3cgcmVzaXplLgotICAgIGdsWFN3YXBCdWZmZXJz KG1fc2hhcmVkUmVzb3VyY2VzLT54MTFEaXNwbGF5KCksIHdpbmRvd0lkKTsKKyAgICBYRmx1c2go bV9zaGFyZWRSZXNvdXJjZXMtPngxMURpc3BsYXkoKSk7CiB9CiAKIHZvaWQgWDExT2ZmU2NyZWVu V2luZG93OjpjcmVhdGVPZmZzY3JlZW5XaW5kb3codWludDMyX3QqIGhhbmRsZUlkKQo= 182047 2013-01-09 19:21:23 -0800 2013-01-09 19:44:20 -0800 patch v2 glxchangesreview.patch text/plain 2272 kalyan.kondapally ZGlmZiAtLWdpdCBTb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCmluZGV4IDVhOGUzMTMuLmZiZmQ0MWMgMTAwNjQ0Ci0tLSBTb3VyY2UvV2ViQ29yZS9DaGFu Z2VMb2cKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDEz LTAxLTA4ICBLb25kYXBhbGx5IEthbHlhbiAgPGthbHlhbi5rb25kYXBhbGx5QGludGVsLmNvbT4K KworICAgICAgICBbRUZMXSBbV2ViR0xdIFJlbW92ZSBHTFggZGVwZW5kZW5jaWVzIGZyb20gWDEx V2luZG93UmVzb3VyY2VzLi4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTEwNjMxOQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgIFRoaXMgcGF0Y2ggcmVtb3ZlcyBHTFggc3BlY2lmaWMgY2FsbHMgZnJvbSBYMTFX aW5kb3dSZXNvdXJjZXMuCisgICAgICAgIFRoaXMgY2xhc3MgaXMgc2hhcmVkIGJ5IGJvdGggR0xY IGFuZCBFR0wgaW1wbGVtZW50YXRpb25zLgorCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3Mv c3VyZmFjZXMvZ2x4L0dMWFN1cmZhY2UuY3BwOgorICAgICAgICAoV2ViQ29yZTo6R0xYVHJhbnNw b3J0U3VyZmFjZTo6c2V0R2VvbWV0cnkpOgorICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL3N1 cmZhY2VzL2dseC9YMTFXaW5kb3dSZXNvdXJjZXMuY3BwOgorICAgICAgICAoV2ViQ29yZTo6WDEx T2ZmU2NyZWVuV2luZG93OjpyZVNpemVXaW5kb3cpOgorCiAyMDEzLTAxLTA4ICBKdXN0aW4gTm92 b3NhZCAgPGp1bm92QGdvb2dsZS5jb20+CgpkaWZmIC0tZ2l0IFNvdXJjZS9XZWJDb3JlL3BsYXRm b3JtL2dyYXBoaWNzL3N1cmZhY2VzL2dseC9HTFhTdXJmYWNlLmNwcCBTb3VyY2UvV2ViQ29yZS9w bGF0Zm9ybS9ncmFwaGljcy9zdXJmYWNlcy9nbHgvR0xYU3VyZmFjZS5jcHAKaW5kZXggMGM3NzQ2 ZS4uZjUwMjM1NyAxMDA2NDQKLS0tIFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3N1 cmZhY2VzL2dseC9HTFhTdXJmYWNlLmNwcAorKysgU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3Jh cGhpY3Mvc3VyZmFjZXMvZ2x4L0dMWFN1cmZhY2UuY3BwCkBAIC03NSw2ICs3NSw4IEBAIHZvaWQg R0xYVHJhbnNwb3J0U3VyZmFjZTo6c2V0R2VvbWV0cnkoY29uc3QgSW50UmVjdCYgbmV3UmVjdCkK IHsKICAgICBHTFBsYXRmb3JtU3VyZmFjZTo6c2V0R2VvbWV0cnkobmV3UmVjdCk7CiAgICAgbV9u YXRpdmVSZXNvdXJjZS0+cmVTaXplV2luZG93KG5ld1JlY3QsIG1fZHJhd2FibGUpOworICAgIC8v IEZvcmNlIHJlc2l6ZSBvZiBHTCBzdXJmYWNlIGFmdGVyIHdpbmRvdyByZXNpemUuCisgICAgZ2xY U3dhcEJ1ZmZlcnMoc2hhcmVkRGlzcGxheSgpLCBtX2RyYXdhYmxlKTsKIH0KIAogdm9pZCBHTFhU cmFuc3BvcnRTdXJmYWNlOjpzd2FwQnVmZmVycygpCmRpZmYgLS1naXQgU291cmNlL1dlYkNvcmUv cGxhdGZvcm0vZ3JhcGhpY3Mvc3VyZmFjZXMvZ2x4L1gxMVdpbmRvd1Jlc291cmNlcy5jcHAgU291 cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvc3VyZmFjZXMvZ2x4L1gxMVdpbmRvd1Jlc291 cmNlcy5jcHAKaW5kZXggODAwOGFmYi4uNWE1N2E4MSAxMDA2NDQKLS0tIFNvdXJjZS9XZWJDb3Jl L3BsYXRmb3JtL2dyYXBoaWNzL3N1cmZhY2VzL2dseC9YMTFXaW5kb3dSZXNvdXJjZXMuY3BwCisr KyBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9zdXJmYWNlcy9nbHgvWDExV2luZG93 UmVzb3VyY2VzLmNwcApAQCAtNTAsOSArNTAsNyBAQCBYMTFPZmZTY3JlZW5XaW5kb3c6On5YMTFP ZmZTY3JlZW5XaW5kb3coKQogdm9pZCBYMTFPZmZTY3JlZW5XaW5kb3c6OnJlU2l6ZVdpbmRvdyhj b25zdCBJbnRSZWN0JiBuZXdSZWN0LCBjb25zdCB1aW50MzJfdCB3aW5kb3dJZCkKIHsKICAgICBY UmVzaXplV2luZG93KG1fc2hhcmVkUmVzb3VyY2VzLT54MTFEaXNwbGF5KCksIHdpbmRvd0lkLCBu ZXdSZWN0LndpZHRoKCksIG5ld1JlY3QuaGVpZ2h0KCkpOwotCi0gICAgLy8gRm9yY2UgcmVzaXpl IG9mIEdMIHN1cmVmYWNlIGFmdGVyIHdpbmRvdyByZXNpemUuCi0gICAgZ2xYU3dhcEJ1ZmZlcnMo bV9zaGFyZWRSZXNvdXJjZXMtPngxMURpc3BsYXkoKSwgd2luZG93SWQpOworICAgIFhGbHVzaCht X3NoYXJlZFJlc291cmNlcy0+eDExRGlzcGxheSgpKTsKIH0KIAogdm9pZCBYMTFPZmZTY3JlZW5X aW5kb3c6OmNyZWF0ZU9mZnNjcmVlbldpbmRvdyh1aW50MzJfdCogaGFuZGxlSWQpCg==