Feature/uniform location by Mee-gu · Pull Request #113 · minggo/new-renderer

Mee-gu · 2019-01-03T09:05:44Z

update uniform location

minggo · 2019-01-03T09:44:38Z

src/backend/BindGroup.h

 private:
-    std::unordered_map<std::string, UniformInfo> _uniformInfos;
+    std::unordered_map<int, UniformInfo> _vertexUniformInfos;
+    std::unordered_map<int, UniformInfo> _fragUniformInfos;


Can use std::vector instead, and reserve 3/5 elements at start.

Mee-gu · 2019-01-04T08:07:58Z

update review

minggo · 2019-01-04T08:20:01Z

src/backend/Program.cpp

+{
+    CC_SAFE_RELEASE(_vertexShader);
+    CC_SAFE_RELEASE(_fragmentShader);
+    _textureInfos.clear();


it is not needed

minggo · 2019-01-04T08:28:09Z

src/backend/metal/ProgramMTL.h

+{
+public:
+    ProgramMTL(ShaderModule* vs, ShaderModule* fs);
+    virtual ~ProgramMTL();


Don't have to be virtual.

minggo · 2019-01-04T08:30:58Z

src/backend/metal/DeviceMTL.mm


+Program* DeviceMTL::createProgram(ShaderModule* vs, ShaderModule* fs)
+{
+    return new (std::nothrow) ProgramMTL(vs, fs);


create function should be auto released.

minggo · 2019-01-04T08:31:31Z

src/backend/Program.h

+        std::vector<Texture*> textures;
+    };
+
+    virtual int getUniformLocation(const std::string& uniform) = 0;


please add const

and i think should use getVertexUniformLoacation and getFragmentUniformLoacation instead

minggo · 2019-01-04T08:35:22Z

src/backend/metal/ProgramMTL.h

+    virtual void setVertexUniform(int location, void* data, uint32_t size) override;
+    virtual void setFragmentUniform(int location, void* data, uint32_t size) override;
+
+    void fillUniformBuffer(uint8_t* buffer, uint32_t offset, void* uniformData, uint32_t uniformSize) const;


const is not needed

Mee-gu · 2019-01-06T08:18:15Z

update review

minggo · 2019-01-07T01:36:27Z

src/backend/RenderPipeline.h

+    RenderPipeline(Program* program);
+    virtual ~RenderPipeline();
+
+    virtual Program* getProgram() { return _program; }


please add const

minggo · 2019-01-07T01:37:02Z

src/backend/RenderPipeline.h

 {
+public:
+    RenderPipeline(Program* program);
+    virtual ~RenderPipeline();


does it need to be public?

minggo · 2019-01-07T01:48:44Z

src/backend/opengl/ProgramGL.cpp

+    _vertexTextureInfos.reserve(_maxTextureLocation);
+    _vertexTextureInfos.resize(_maxTextureLocation);
+    _fragTextureInfos.reserve(_maxTextureLocation);
+    _fragTextureInfos.resize(_maxTextureLocation);


don't have to use reserve and resize at the same time

minggo · 2019-01-07T02:01:48Z

src/backend/opengl/ProgramGL.cpp

+        CC_SAFE_RETAIN(_vertexShaderModule);
+        CC_SAFE_RETAIN(_fragmentShaderModule);
+        return;
+    }


can not understand the logic

minggo · 2019-01-07T02:09:32Z

src/backend/opengl/ProgramGL.cpp

+        return;
+
+    glUseProgram(_program);
+    for(const auto& uniforn : _uniformInfos)


can optimize it, don't have to loop every time

Mee-gu · 2019-01-07T07:39:49Z

add ProgramCache

Mee-gu · 2019-01-08T02:23:17Z

update ProgramCache

minggo · 2019-01-08T02:30:54Z

test/macOS/main.mm

            dt = std::chrono::duration_cast<std::chrono::microseconds>(now - prevTime).count() / 1000000.f;
        }
-
+        cocos2d::backend::ProgramCache::destroyInstance();


should remove it

Mee-gu added 3 commits January 3, 2019 16:56

[Feature] update uniform location

d094d41

[Feature] remove unneeded code

6b15161

[Feature] add program files

01218b9

minggo reviewed Jan 3, 2019

View reviewed changes

[Feature] remove BindGroup

17c23e4

[BugFix] fix reference retain

9786a45

minggo reviewed Jan 4, 2019

View reviewed changes

Mee-gu added 2 commits January 6, 2019 16:08

[Feature] update uniform location

999198e

[Feature] remove unneeded code

2152f8e

minggo reviewed Jan 7, 2019

View reviewed changes

Mee-gu added 3 commits January 7, 2019 11:18

[Feature] use unordered_map to simply code

c38092c

[Feature] add ProgramCache

64706e1

[Feature] remove unneeded code

10c0698

[Feature] update ProgramCache

2063ed7

minggo reviewed Jan 8, 2019

View reviewed changes

Conversation

Mee-gu commented Jan 3, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mee-gu commented Jan 4, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mee-gu commented Jan 6, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mee-gu commented Jan 7, 2019

Uh oh!

Mee-gu commented Jan 8, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants