Bug 58905 - [Qt][Texmap] Do not create a texture if it will not be used.
Summary: [Qt][Texmap] Do not create a texture if it will not be used.
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks: 47068
  Show dependency tree / graph
 
Reported: 2011-04-19 10:46 PDT by Young Han Lee
Modified: 2011-05-30 07:29 PDT (History)
8 users (show)

See Also:

Attachments
Patch (3.20 KB, patch)
2011-04-19 10:51 PDT, Young Han Lee 		laszlo.gombos: review-
Details | Formatted Diff | Diff
Patch (3.17 KB, patch)
2011-04-27 07:50 PDT, Young Han Lee 		tonikitoo: review-
Details | Formatted Diff | Diff
Fixed (3.16 KB, patch)
2011-04-27 20:32 PDT, Young Han Lee 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Young Han Lee 2011-04-19 10:46:56 PDT 
[Qt][Texmap] Do not create a texture if it will not be used.
Comment 1 Young Han Lee 2011-04-19 10:51:19 PDT 
Created attachment 90215 [details]
Patch
Comment 2 Young Han Lee 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.
Comment 3 Laszlo Gombos 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() ?
Comment 4 Young Han Lee 2011-04-27 07:50:24 PDT 
Created attachment 91281 [details]
Patch

haveContent() looks better! thx.
Comment 5 Antonio Gomes 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.
Comment 6 Young Han Lee 2011-04-27 20:32:27 PDT 
Created attachment 91417 [details]
Fixed

Fixed. I didn't find a better name.
Comment 7 Alexis Menard (darktears) 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?
Comment 8 Young Han Lee 2011-05-30 07:28:16 PDT 
This problem is resolved by https://bugs.webkit.org/show_bug.cgi?id=60439