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.
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
Keywords: Qt, QtTriaged
Depends on:
Blocks: 47068
  Show dependency treegraph
Reported: 2011-04-19 10:46 PDT by Young Han Lee
Modified: 2011-05-30 07:29 PDT (History)
8 users (show)

See Also:

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

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]
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]

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.


> 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]

haveContent() looks better! thx.
Comment 5 Antonio Gomes 2011-04-27 20:10:41 PDT
Comment on attachment 91281 [details]

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. I didn't find a better name.
Comment 7 Alexis Menard (darktears) 2011-05-03 16:34:17 PDT
Comment on attachment 91417 [details]

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