Summary: | [Qt] [WK2] Webkit should release TextureMapper GL objects if page paint node is deallocated. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Viatcheslav Ostapenko <ostap73> | ||||||||||
Component: | WebKit Qt | Assignee: | Viatcheslav Ostapenko <ostap73> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | igor.oliveira, noam, webkit.review.bot, zoltan | ||||||||||
Priority: | P3 | Keywords: | Qt | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 74176 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
Viatcheslav Ostapenko
2011-12-01 14:36:33 PST
Created attachment 117497 [details]
patch
Comment on attachment 117497 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=117497&action=review See comments. Question, what are we doing with directly-composited images? Would that come in a different patch? > Source/WebCore/ChangeLog:3 > + [Qt] [WK2] Webkit should TextureMapper GL objects if page paint node is deallocated. I think this is missing a verb (release?). > Source/WebKit2/WebProcess/WebCoreSupport/WebGraphicsLayer.cpp:579 > +void WebGraphicsLayer::reCreateTiledBackingStore() Let's call it recreateTiledBackingStoreIfNeeded > Source/WebKit2/WebProcess/WebCoreSupport/WebGraphicsLayer.cpp:591 > +} Don't you need to regenerate the content as well, e.g. setNeedsDisplay ? > Source/WebKit2/WebProcess/WebPage/LayerTreeHost.h:87 > + virtual void needTileBuffersRecreate() { } purgeTiledBackingStores (?) > Source/WebKit2/WebProcess/WebPage/qt/LayerTreeHostQt.cpp:69 > + , m_needTileBuffersRecreate(false) m_shouldRecreateTileBuffers (In reply to comment #2) > (From update of attachment 117497 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=117497&action=review > > See comments. > Question, what are we doing with directly-composited images? > Would that come in a different patch? No. For some reason I thought that tiles are used for directly-composited images also. I'll figure it out. > > Source/WebKit2/WebProcess/WebCoreSupport/WebGraphicsLayer.cpp:591 > > +} > Don't you need to regenerate the content as well, e.g. setNeedsDisplay ? IMHO it called somewhere else on page getting visible, but I think it doesn't harm to call it here also. Created attachment 118504 [details]
Added cleaning of direct composited images
Comment on attachment 118504 [details] Added cleaning of direct composited images View in context: https://bugs.webkit.org/attachment.cgi?id=118504&action=review You're using too many names to describe the same thing. How about: purgeNodeTexturesRecursive PurgeBackingStores purgeGLResouces recreateBackingStoreIfNeeded purgeBackingStore m_shouldRecreateBackingStore > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:74 > + // Trigger setting of correct visibility flags after everything was allocated and initialized Period at end of sentence. > Source/WebKit2/WebProcess/WebCoreSupport/WebGraphicsLayer.cpp:591 > + if (m_layerInfo.imageBackingStoreID) { > + layerTreeTileClient()->releaseImageBackingStore(m_layerInfo.imageBackingStoreID); > + m_layerInfo.imageBackingStoreID = 0; > + } Early return > Source/WebKit2/WebProcess/WebCoreSupport/WebGraphicsLayer.cpp:596 > + Newline unnecessary > Source/WebKit2/WebProcess/WebPage/qt/LayerTreeHostQt.cpp:440 > + if (m_shouldRecreateTileBuffers) { Early return. Created attachment 118522 [details]
Names and style fixed.
Comment on attachment 118522 [details] Names and style fixed. View in context: https://bugs.webkit.org/attachment.cgi?id=118522&action=review Please fix typo, and regenerate Changelogs :) > Source/WebKit2/UIProcess/LayerTreeHostProxy.h:58 > + void purgeGLResouces(); you mean purgeGLResources? (typo) (In reply to comment #7) > (From update of attachment 118522 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=118522&action=review > > Please fix typo, and regenerate Changelogs :) > > > Source/WebKit2/UIProcess/LayerTreeHostProxy.h:58 > > + void purgeGLResouces(); > > you mean purgeGLResources? > (typo) Uff! I hate copy&paste ;) > > > Source/WebKit2/UIProcess/LayerTreeHostProxy.h:58
> > > + void purgeGLResouces();
> >
> > you mean purgeGLResources?
> > (typo)
>
> Uff!
> I hate copy&paste ;)
Just checking that you're alert...
Created attachment 118527 [details]
Resouces fixed
Comment on attachment 118527 [details] Resouces fixed Clearing flags on attachment: 118527 Committed r102435: <http://trac.webkit.org/changeset/102435> All reviewed patches have been landed. Closing bug. |