Summary: | [Qt] GraphicsContext3D internal buffers are not freed | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jarkko Sakkinen <jarkko.j.sakkinen> | ||||||||
Component: | WebKit Qt | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | benjamin, commit-queue | ||||||||
Priority: | P2 | Keywords: | Qt, QtTriaged | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Jarkko Sakkinen
2011-03-31 01:06:40 PDT
Created attachment 87685 [details]
fix
Comment on attachment 87685 [details] 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(). Created attachment 88264 [details]
QString::fromUtf8()
Comment on attachment 88264 [details] 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. Created attachment 89460 [details] free internal buffers This is Jarkkos patch without the extra compiler warning cleanup, which is now in a patch on bug #58478 Comment on attachment 89460 [details]
free internal buffers
Looks good.
(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. Comment on attachment 89460 [details] free internal buffers Clearing flags on attachment: 89460 Committed r83834: <http://trac.webkit.org/changeset/83834> All reviewed patches have been landed. Closing bug. |