Bug 58905

Summary: [Qt][Texmap] Do not create a texture if it will not be used.
Product: WebKit Reporter: Young Han Lee <joybro201>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: benjamin, cmarrin, eric, jamesr, kbr, laszlo.gombos, noam, skyul
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 47068    
Attachments:
Description Flags
Patch
laszlo.gombos: review-
Patch
tonikitoo: review-
Fixed none

Young Han Lee
Reported 2011-04-19 10:46:56 PDT
[Qt][Texmap] Do not create a texture if it will not be used.
Attachments
Patch (3.20 KB, patch)
2011-04-19 10:51 PDT, Young Han Lee 		laszlo.gombos: review-
DetailsFormatted DiffDiff
Patch (3.17 KB, patch)
2011-04-27 07:50 PDT, Young Han Lee 		tonikitoo: review-
DetailsFormatted DiffDiff
Fixed (3.16 KB, patch)
2011-04-27 20:32 PDT, Young Han Lee 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Young Han Lee
Comment 1 2011-04-19 10:51:19 PDT
Created attachment 90215 [details] Patch
Young Han Lee
Comment 2 2011-04-19 10:53:23 PDT
Currently, all TextureMapperNodes create it's own texture even if the texture will not be used. A lot of memory is being wasted due to this. Whether a TextureMapperNode will be painted or not is determined in the sync-phase, before a texture is created. So, do not create a texture if it is not really needed.
Laszlo Gombos
Comment 3 2011-04-26 17:13:03 PDT
Comment on attachment 90215 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=90215&action=review Overall change looks good, r- to fix the typo and change come up with a better function name. > Source/WebCore/ChangeLog:14 > + No new tests required as no functionaliry changes. Typo. > Source/WebCore/platform/graphics/texmap/TextureMapperNode.h:176 > + inline bool shouldBePainted() const I think we should have a more descriptive name - what about haveContent() ?
Young Han Lee
Comment 4 2011-04-27 07:50:24 PDT
Created attachment 91281 [details] Patch haveContent() looks better! thx.
Antonio Gomes
Comment 5 2011-04-27 20:10:41 PDT
Comment on attachment 91281 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91281&action=review > Source/WebCore/platform/graphics/texmap/TextureMapperNode.h:176 > + inline bool haveContent() const it should be named hasContent, or something better and more descriptive.
Young Han Lee
Comment 6 2011-04-27 20:32:27 PDT
Created attachment 91417 [details] Fixed Fixed. I didn't find a better name.
Alexis Menard (darktears)
Comment 7 2011-05-03 16:34:17 PDT
Comment on attachment 91417 [details] Fixed View in context: https://bugs.webkit.org/attachment.cgi?id=91417&action=review > Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:859 > + if (m_state.dirty && hasContent()) I'm wondering if that check should be done before. Why entering here if anyway there is nothing to render?
Young Han Lee
Comment 8 2011-05-30 07:28:16 PDT
This problem is resolved by https://bugs.webkit.org/show_bug.cgi?id=60439
Note You need to log in before you can comment on or make changes to this bug.