57530 2011-03-31 01:06:40 -0700 [Qt] GraphicsContext3D internal buffers are not freed 2011-04-14 04:01:34 -0700 1 1 1 Unclassified WebKit WebKit Qt 528+ (Nightly build) All All RESOLVED FIXED Qt, QtTriaged P2 Normal --- 0 jarkko.j.sakkinen webkit-unassigned benjamin commit-queue oldest_to_newest 376984 0 jarkko.j.sakkinen 2011-03-31 01:06:40 -0700 There's code for destroying internal FBO, color texture and depth buffer from the code. These resouces aren't freed when shader OpenGL context is created with the viewport. 377011 1 87685 jarkko.j.sakkinen 2011-03-31 02:41:55 -0700 Created attachment 87685 fix 379488 2 87685 benjamin 2011-04-05 06:02:36 -0700 Comment on attachment 87685 fix View in context: https://bugs.webkit.org/attachment.cgi?id=87685&action=review > Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:597 > + void* addr = m_glWidget->context()->getProcAddress(QString::fromAscii(nameWithExt.utf8().data())); QString::fromAscii() is almost never valid inside a library. It does not actually convert from Ascii, but from the default codec, which can be set to anything by the user code. You should use QString::fromLatin1() or QLatin1String() or in thisn case: QString::fromUtf8(). 379685 3 88264 jarkko.j.sakkinen 2011-04-05 09:56:19 -0700 Created attachment 88264 QString::fromUtf8() 380443 4 88264 benjamin 2011-04-06 09:06:45 -0700 Comment on attachment 88264 QString::fromUtf8() View in context: https://bugs.webkit.org/attachment.cgi?id=88264&action=review > Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:455 > + m_glWidget->makeCurrent(); > + if (m_glWidget->isValid()) { > + ::glDeleteTextures(1, &m_texture); > + deleteRenderbuffers(1, &m_depthBuffer); > + deleteFramebuffers(1, &m_canvasFbo); > + } Please resubmit the patch with just this. > Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:597 > + void* addr = m_glWidget->context()->getProcAddress(QString::fromUtf8(nameWithExt.utf8().data())); Actually, looking at it, shouldn't it be QString(nameWithExt)? I don't see why we should pass by UTF-8 to convert for WTF::String to QString. You should commit this separately. It seems unrelated with the resource cleaning. > Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:665 > + if ((width == m_currentWidth && height == m_currentHeight) || (!m_internal)) No problem with that but you can commit that separately, with the above change, in a code cleaning patch. 385243 5 89460 rectalogic 2011-04-13 14:32:54 -0700 Created attachment 89460 free internal buffers This is Jarkkos patch without the extra compiler warning cleanup, which is now in a patch on bug #58478 385341 6 89460 benjamin 2011-04-13 16:19:07 -0700 Comment on attachment 89460 free internal buffers Looks good. 385343 7 benjamin 2011-04-13 16:20:16 -0700 (In reply to comment #5) > Created an attachment (id=89460) [details] > free internal buffers > > This is Jarkkos patch without the extra compiler warning cleanup, which is now in a patch on bug #58478 Good job doing the updated patch. Jarkko is on vacation IIRC. 385631 8 89460 commit-queue 2011-04-14 04:01:31 -0700 Comment on attachment 89460 free internal buffers Clearing flags on attachment: 89460 Committed r83834: <http://trac.webkit.org/changeset/83834> 385632 9 commit-queue 2011-04-14 04:01:34 -0700 All reviewed patches have been landed. Closing bug. 87685 2011-03-31 02:41:55 -0700 2011-04-05 09:56:19 -0700 fix 57530.diff text/plain 2234 jarkko.j.sakkinen ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBjNTRhMmUxLi44ZWQzMDMzIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUg QEAKKzIwMTEtMDMtMzEgIEphcmtrbyBTYWtraW5lbiAgPGphcmtrby5qLnNha2tpbmVuQGdtYWls LmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBb UXRdIEdyYXBoaWNzQ29udGV4dDNEIGludGVybmFsIGJ1ZmZlcnMgYXJlIG5vdCBmcmVlZAorICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTc1MzAKKworICAg ICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL3F0L0dyYXBoaWNzQ29udGV4dDNEUXQuY3BwOgorICAg ICAgICAoV2ViQ29yZTo6R3JhcGhpY3NDb250ZXh0M0RJbnRlcm5hbDo6fkdyYXBoaWNzQ29udGV4 dDNESW50ZXJuYWwpOgorICAgICAgICAoV2ViQ29yZTo6R3JhcGhpY3NDb250ZXh0M0RJbnRlcm5h bDo6Z2V0UHJvY0FkZHJlc3MpOgorICAgICAgICAoV2ViQ29yZTo6R3JhcGhpY3NDb250ZXh0M0Q6 OnJlc2hhcGUpOgorCiAyMDExLTAzLTMxICBCZW5qYW1pbiBLYWxtYW4gIDxrYWxtYW5AY2hyb21p dW0ub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IE9qYW4gVmFmYWkuCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9xdC9HcmFwaGljc0NvbnRleHQzRFF0LmNw cCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3F0L0dyYXBoaWNzQ29udGV4dDNE UXQuY3BwCmluZGV4IDRkYWE0ZGMuLjQzMmExNWEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3Jl L3BsYXRmb3JtL2dyYXBoaWNzL3F0L0dyYXBoaWNzQ29udGV4dDNEUXQuY3BwCisrKyBiL1NvdXJj ZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3F0L0dyYXBoaWNzQ29udGV4dDNEUXQuY3BwCkBA IC00NDcsNiArNDQ3LDEyIEBAIEdyYXBoaWNzQ29udGV4dDNESW50ZXJuYWw6OkdyYXBoaWNzQ29u dGV4dDNESW50ZXJuYWwoR3JhcGhpY3NDb250ZXh0M0Q6OkF0dHJpYnV0CiAKIEdyYXBoaWNzQ29u dGV4dDNESW50ZXJuYWw6On5HcmFwaGljc0NvbnRleHQzREludGVybmFsKCkKIHsKKyAgICBtX2ds V2lkZ2V0LT5tYWtlQ3VycmVudCgpOworICAgIGlmIChtX2dsV2lkZ2V0LT5pc1ZhbGlkKCkpIHsK KyAgICAgICAgOjpnbERlbGV0ZVRleHR1cmVzKDEsICZtX3RleHR1cmUpOworICAgICAgICBkZWxl dGVSZW5kZXJidWZmZXJzKDEsICZtX2RlcHRoQnVmZmVyKTsKKyAgICAgICAgZGVsZXRlRnJhbWVi dWZmZXJzKDEsICZtX2NhbnZhc0Zibyk7CisgICAgfQogICAgIGRlbGV0ZSBtX2dsV2lkZ2V0Owog ICAgIG1fZ2xXaWRnZXQgPSAwOwogfQpAQCAtNTg4LDcgKzU5NCw3IEBAIHZvaWQqIEdyYXBoaWNz Q29udGV4dDNESW50ZXJuYWw6OmdldFByb2NBZGRyZXNzKGNvbnN0IFN0cmluZyYgcHJvYykKICAg ICBmb3IgKGludCBpID0gMDsgaSA8IDM7IGkrKykgewogICAgICAgICBTdHJpbmcgbmFtZVdpdGhF eHQgPSBwcm9jICsgZXh0W2ldOwogCi0gICAgICAgIHZvaWQqIGFkZHIgPSBtX2dsV2lkZ2V0LT5j b250ZXh0KCktPmdldFByb2NBZGRyZXNzKG5hbWVXaXRoRXh0LnV0ZjgoKS5kYXRhKCkpOworICAg ICAgICB2b2lkKiBhZGRyID0gbV9nbFdpZGdldC0+Y29udGV4dCgpLT5nZXRQcm9jQWRkcmVzcyhR U3RyaW5nOjpmcm9tQXNjaWkobmFtZVdpdGhFeHQudXRmOCgpLmRhdGEoKSkpOwogICAgICAgICBp ZiAoYWRkcikgCiAgICAgICAgICAgICByZXR1cm4gYWRkcjsKICAgICB9CkBAIC02NTYsNyArNjYy LDcgQEAgUGFzc1JlZlB0cjxJbWFnZURhdGE+IEdyYXBoaWNzQ29udGV4dDNEOjpwYWludFJlbmRl cmluZ1Jlc3VsdHNUb0ltYWdlRGF0YSgpCiAKIHZvaWQgR3JhcGhpY3NDb250ZXh0M0Q6OnJlc2hh cGUoaW50IHdpZHRoLCBpbnQgaGVpZ2h0KQogewotICAgIGlmICh3aWR0aCA9PSBtX2N1cnJlbnRX aWR0aCAmJiBoZWlnaHQgPT0gbV9jdXJyZW50SGVpZ2h0IHx8ICghbV9pbnRlcm5hbCkpCisgICAg aWYgKCh3aWR0aCA9PSBtX2N1cnJlbnRXaWR0aCAmJiBoZWlnaHQgPT0gbV9jdXJyZW50SGVpZ2h0 KSB8fCAoIW1faW50ZXJuYWwpKQogICAgICAgICByZXR1cm47CiAKICAgICBtX2N1cnJlbnRXaWR0 aCA9IHdpZHRoOwo= 88264 2011-04-05 09:56:19 -0700 2011-04-06 09:06:45 -0700 QString::fromUtf8() 57530.asc text/plain 2228 jarkko.j.sakkinen ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAwYTgwNTdjLi5iNjZlYTQ3IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUg QEAKKzIwMTEtMDQtMDUgIEphcmtrbyBTYWtraW5lbiAgPGphcmtrby5qLnNha2tpbmVuQGdtYWls LmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBb UXRdIEdyYXBoaWNzQ29udGV4dDNEIGludGVybmFsIGJ1ZmZlcnMgYXJlIG5vdCBmcmVlZAorICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTc1MzAKKworICAg ICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL3F0L0dyYXBoaWNzQ29udGV4dDNEUXQuY3BwOgorICAg ICAgICAoV2ViQ29yZTo6R3JhcGhpY3NDb250ZXh0M0RJbnRlcm5hbDo6fkdyYXBoaWNzQ29udGV4 dDNESW50ZXJuYWwpOgorICAgICAgICAoV2ViQ29yZTo6R3JhcGhpY3NDb250ZXh0M0RJbnRlcm5h bDo6Z2V0UHJvY0FkZHJlc3MpOgorICAgICAgICAoV2ViQ29yZTo6R3JhcGhpY3NDb250ZXh0M0Q6 OnJlc2hhcGUpOgorCiAyMDExLTA0LTAxICBKZXIgTm9ibGUgIDxqZXIubm9ibGVAYXBwbGUuY29t PgogCiAgICAgICAgIFJldmlld2VkIGJ5IERhcmluIEFkbGVyLgpkaWZmIC0tZ2l0IGEvU291cmNl L1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvcXQvR3JhcGhpY3NDb250ZXh0M0RRdC5jcHAgYi9T b3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9xdC9HcmFwaGljc0NvbnRleHQzRFF0LmNw cAppbmRleCA0ZGFhNGRjLi4wNmY5M2FjIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0 Zm9ybS9ncmFwaGljcy9xdC9HcmFwaGljc0NvbnRleHQzRFF0LmNwcAorKysgYi9Tb3VyY2UvV2Vi Q29yZS9wbGF0Zm9ybS9ncmFwaGljcy9xdC9HcmFwaGljc0NvbnRleHQzRFF0LmNwcApAQCAtNDQ3 LDYgKzQ0NywxMiBAQCBHcmFwaGljc0NvbnRleHQzREludGVybmFsOjpHcmFwaGljc0NvbnRleHQz REludGVybmFsKEdyYXBoaWNzQ29udGV4dDNEOjpBdHRyaWJ1dAogCiBHcmFwaGljc0NvbnRleHQz REludGVybmFsOjp+R3JhcGhpY3NDb250ZXh0M0RJbnRlcm5hbCgpCiB7CisgICAgbV9nbFdpZGdl dC0+bWFrZUN1cnJlbnQoKTsKKyAgICBpZiAobV9nbFdpZGdldC0+aXNWYWxpZCgpKSB7CisgICAg ICAgIDo6Z2xEZWxldGVUZXh0dXJlcygxLCAmbV90ZXh0dXJlKTsKKyAgICAgICAgZGVsZXRlUmVu ZGVyYnVmZmVycygxLCAmbV9kZXB0aEJ1ZmZlcik7CisgICAgICAgIGRlbGV0ZUZyYW1lYnVmZmVy cygxLCAmbV9jYW52YXNGYm8pOworICAgIH0KICAgICBkZWxldGUgbV9nbFdpZGdldDsKICAgICBt X2dsV2lkZ2V0ID0gMDsKIH0KQEAgLTU4OCw3ICs1OTQsNyBAQCB2b2lkKiBHcmFwaGljc0NvbnRl eHQzREludGVybmFsOjpnZXRQcm9jQWRkcmVzcyhjb25zdCBTdHJpbmcmIHByb2MpCiAgICAgZm9y IChpbnQgaSA9IDA7IGkgPCAzOyBpKyspIHsKICAgICAgICAgU3RyaW5nIG5hbWVXaXRoRXh0ID0g cHJvYyArIGV4dFtpXTsKIAotICAgICAgICB2b2lkKiBhZGRyID0gbV9nbFdpZGdldC0+Y29udGV4 dCgpLT5nZXRQcm9jQWRkcmVzcyhuYW1lV2l0aEV4dC51dGY4KCkuZGF0YSgpKTsKKyAgICAgICAg dm9pZCogYWRkciA9IG1fZ2xXaWRnZXQtPmNvbnRleHQoKS0+Z2V0UHJvY0FkZHJlc3MoUVN0cmlu Zzo6ZnJvbVV0ZjgobmFtZVdpdGhFeHQudXRmOCgpLmRhdGEoKSkpOwogICAgICAgICBpZiAoYWRk cikgCiAgICAgICAgICAgICByZXR1cm4gYWRkcjsKICAgICB9CkBAIC02NTYsNyArNjYyLDcgQEAg UGFzc1JlZlB0cjxJbWFnZURhdGE+IEdyYXBoaWNzQ29udGV4dDNEOjpwYWludFJlbmRlcmluZ1Jl c3VsdHNUb0ltYWdlRGF0YSgpCiAKIHZvaWQgR3JhcGhpY3NDb250ZXh0M0Q6OnJlc2hhcGUoaW50 IHdpZHRoLCBpbnQgaGVpZ2h0KQogewotICAgIGlmICh3aWR0aCA9PSBtX2N1cnJlbnRXaWR0aCAm JiBoZWlnaHQgPT0gbV9jdXJyZW50SGVpZ2h0IHx8ICghbV9pbnRlcm5hbCkpCisgICAgaWYgKCh3 aWR0aCA9PSBtX2N1cnJlbnRXaWR0aCAmJiBoZWlnaHQgPT0gbV9jdXJyZW50SGVpZ2h0KSB8fCAo IW1faW50ZXJuYWwpKQogICAgICAgICByZXR1cm47CiAKICAgICBtX2N1cnJlbnRXaWR0aCA9IHdp ZHRoOwo= 89460 2011-04-13 14:32:54 -0700 2011-04-14 04:01:31 -0700 free internal buffers opengl.diff text/plain 1337 rectalogic SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDgzNzYzKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTMgQEAKKzIwMTEtMDQtMDUgIEphcmtrbyBT YWtraW5lbiAgPGphcmtrby5qLnNha2tpbmVuQGdtYWlsLmNvbT4KKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBbUXRdIEdyYXBoaWNzQ29udGV4dDNEIGlu dGVybmFsIGJ1ZmZlcnMgYXJlIG5vdCBmcmVlZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTc1MzAKKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNz L3F0L0dyYXBoaWNzQ29udGV4dDNEUXQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6R3JhcGhpY3ND b250ZXh0M0RJbnRlcm5hbDo6fkdyYXBoaWNzQ29udGV4dDNESW50ZXJuYWwpOgorCiAyMDExLTA0 LTEzICBFcmljIENhcmxzb24gIDxlcmljLmNhcmxzb25AYXBwbGUuY29tPgogCiAgICAgICAgIFJl dmlld2VkIGJ5IEFsZXhleSBQcm9za3VyeWFrb3YuCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9wbGF0 Zm9ybS9ncmFwaGljcy9xdC9HcmFwaGljc0NvbnRleHQzRFF0LmNwcAo9PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBT b3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9xdC9HcmFwaGljc0NvbnRleHQzRFF0LmNw cAkocmV2aXNpb24gODM3NjMpCisrKyBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9x dC9HcmFwaGljc0NvbnRleHQzRFF0LmNwcAkod29ya2luZyBjb3B5KQpAQCAtNDQ3LDYgKzQ0Nywx MiBAQCBHcmFwaGljc0NvbnRleHQzREludGVybmFsOjpHcmFwaGljc0NvbnRlCiAKIEdyYXBoaWNz Q29udGV4dDNESW50ZXJuYWw6On5HcmFwaGljc0NvbnRleHQzREludGVybmFsKCkKIHsKKyAgICBt X2dsV2lkZ2V0LT5tYWtlQ3VycmVudCgpOworICAgIGlmIChtX2dsV2lkZ2V0LT5pc1ZhbGlkKCkp IHsKKyAgICAgICAgOjpnbERlbGV0ZVRleHR1cmVzKDEsICZtX3RleHR1cmUpOworICAgICAgICBk ZWxldGVSZW5kZXJidWZmZXJzKDEsICZtX2RlcHRoQnVmZmVyKTsKKyAgICAgICAgZGVsZXRlRnJh bWVidWZmZXJzKDEsICZtX2NhbnZhc0Zibyk7CisgICAgfQogICAgIGRlbGV0ZSBtX2dsV2lkZ2V0 OwogICAgIG1fZ2xXaWRnZXQgPSAwOwogfQo=