Skip to content

Feature/uniform location#113

Open
Mee-gu wants to merge 11 commits into
minggo:metal-supportfrom
Mee-gu:feature/uniformLocation
Open

Feature/uniform location#113
Mee-gu wants to merge 11 commits into
minggo:metal-supportfrom
Mee-gu:feature/uniformLocation

Conversation

@Mee-gu

@Mee-gu Mee-gu commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

update uniform location

Comment thread src/backend/BindGroup.h Outdated
private:
std::unordered_map<std::string, UniformInfo> _uniformInfos;
std::unordered_map<int, UniformInfo> _vertexUniformInfos;
std::unordered_map<int, UniformInfo> _fragUniformInfos;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@Mee-gu

Mee-gu commented Jan 4, 2019

Copy link
Copy Markdown
Contributor Author

update review

Comment thread src/backend/Program.cpp Outdated
{
CC_SAFE_RELEASE(_vertexShader);
CC_SAFE_RELEASE(_fragmentShader);
_textureInfos.clear();

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is not needed

Comment thread src/backend/metal/ProgramMTL.h Outdated
{
public:
ProgramMTL(ShaderModule* vs, ShaderModule* fs);
virtual ~ProgramMTL();

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't have to be virtual.

Comment thread src/backend/metal/DeviceMTL.mm Outdated

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

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create function should be auto released.

Comment thread src/backend/Program.h Outdated
std::vector<Texture*> textures;
};

virtual int getUniformLocation(const std::string& uniform) = 0;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add const

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and i think should use getVertexUniformLoacation and getFragmentUniformLoacation instead

Comment thread src/backend/metal/ProgramMTL.h Outdated
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;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const is not needed

@Mee-gu

Mee-gu commented Jan 6, 2019

Copy link
Copy Markdown
Contributor Author

update review

Comment thread src/backend/RenderPipeline.h Outdated
RenderPipeline(Program* program);
virtual ~RenderPipeline();

virtual Program* getProgram() { return _program; }

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add const

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

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it need to be public?

Comment thread src/backend/opengl/ProgramGL.cpp Outdated
_vertexTextureInfos.reserve(_maxTextureLocation);
_vertexTextureInfos.resize(_maxTextureLocation);
_fragTextureInfos.reserve(_maxTextureLocation);
_fragTextureInfos.resize(_maxTextureLocation);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/backend/opengl/ProgramGL.cpp Outdated
CC_SAFE_RETAIN(_vertexShaderModule);
CC_SAFE_RETAIN(_fragmentShaderModule);
return;
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can not understand the logic

Comment thread src/backend/opengl/ProgramGL.cpp Outdated
return;

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

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@Mee-gu

Mee-gu commented Jan 7, 2019

Copy link
Copy Markdown
Contributor Author

add ProgramCache

@Mee-gu

Mee-gu commented Jan 8, 2019

Copy link
Copy Markdown
Contributor Author

update ProgramCache

Comment thread test/macOS/main.mm
dt = std::chrono::duration_cast<std::chrono::microseconds>(now - prevTime).count() / 1000000.f;
}

cocos2d::backend::ProgramCache::destroyInstance();

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should remove it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants