Bug 82049 - [Qt][WK2] Make TextureMapperShaderManager::getShaderProgram() not be a template.
Summary: [Qt][WK2] Make TextureMapperShaderManager::getShaderProgram() not be a template.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zeno Albisser
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-23 06:25 PDT by Zeno Albisser
Modified: 2012-03-24 01:38 PDT (History)
2 users (show)

See Also:


Attachments
backtrace of the assert. (12.24 KB, text/plain)
2012-03-23 06:27 PDT, Zeno Albisser
no flags Details
patch for review. (5.24 KB, patch)
2012-03-23 06:43 PDT, Zeno Albisser
no flags Details | Formatted Diff | Diff
patch for review. - fixed issues as requested. (5.29 KB, patch)
2012-03-23 09:07 PDT, Zeno Albisser
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zeno Albisser 2012-03-23 06:25:46 PDT
When building debug with clang on mac, this template function leads to an assert in HashTable.
When building with gcc instead, the problem could not be reproduced.
This even was the case if the function was not using the template parameter at all.
So just having this function being a template triggered what seems to be a compiler bug.
Comment 1 Zeno Albisser 2012-03-23 06:27:06 PDT
Created attachment 133471 [details]
backtrace of the assert.

For the sake of completeness I am adding a backtrace to the bugreport.
Comment 2 Zeno Albisser 2012-03-23 06:43:03 PDT
Created attachment 133474 [details]
patch for review.
Comment 3 Noam Rosenthal 2012-03-23 07:11:12 PDT
Comment on attachment 133474 [details]
patch for review.

View in context: https://bugs.webkit.org/attachment.cgi?id=133474&action=review

> Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.h:123
> +        if (shaderType == Simple) {
> +            RefPtr<TextureMapperShaderProgram> t = TextureMapperShaderProgramSimple::create();
> +            m_textureMapperShaderProgramMap.add(shaderType, t);
> +            return t;
> +        }
> +
> +        if (shaderType == OpacityAndMask) {
> +            RefPtr<TextureMapperShaderProgram> t = TextureMapperShaderProgramOpacityAndMask::create();
> +            m_textureMapperShaderProgramMap.add(shaderType, t);
> +            return t;
> +        }

This should be a switch, with only the different lines in it. The add and return can go outside the switch.

> Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.h:129
> +    typedef HashMap<int, RefPtr<TextureMapperShaderProgram> > TextureMapperShaderProgramMap;

Why not keep the key as ShaderType?
Comment 4 Zeno Albisser 2012-03-23 09:07:12 PDT
Created attachment 133497 [details]
patch for review. - fixed issues as requested.
Comment 5 Zeno Albisser 2012-03-24 01:38:25 PDT
Comment on attachment 133497 [details]
patch for review. - fixed issues as requested.

Clearing flags on attachment: 133497

Committed r111986: <http://trac.webkit.org/changeset/111986>
Comment 6 Zeno Albisser 2012-03-24 01:38:32 PDT
All reviewed patches have been landed.  Closing bug.