Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NUI] AddPrecompileShader() for precompile shader #6298

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

tscholb
Copy link
Contributor

@tscholb tscholb commented Sep 2, 2024

@TizenAPI-Bot
Copy link
Collaborator

Internal API Changed

Added: 1, Removed: 0, Changed: 0

Added

+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Boolean Tizen.NUI.VisualFactory::AddPrecompileShader(Tizen.NUI.PropertyMap)

/// VisualFactory.Instance.UsePreCompiledShader();
///
/// <param name="option">The map contains the shader option for precompiling.</param>
[EditorBrowsable(EditorBrowsableState.Never)]
Copy link
Contributor

Choose a reason for hiding this comment

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

If this method really needs the boolean return, could you leave a description about the return meaning as well?
Like,

Suggested change
[EditorBrowsable(EditorBrowsableState.Never)]
/// <return>True if the pre-compiled shader is added, otherwise false.</return>
[EditorBrowsable(EditorBrowsableState.Never)]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

감사합니다.
리뷰 남겨주신대로 수정 완료했습니다 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

빠른 반영 감사합니다~

Copy link
Contributor

@taehyub taehyub left a comment

Choose a reason for hiding this comment

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

 (example)
 PropertyMap imageShader = new PropertyMap();
 imageShader.Add("shaderType", new PropertyValue("image"));
 imageShader.Add("shaderOption", new PropertyValue(new PropertyMap().Add("ROUNDED_CORNER", new PropertyValue(true))
                                                                     .Add("MASKING", new PropertyValue(true))));

 PropertyMap textShader = new PropertyMap();
 textShader.Add("shaderType", new PropertyValue("text"));

 VisualFactory.Instance.AddPrecompileShader(imageShader);
 VisualFactory.Instance.AddPrecompileShader(textShader);
 VisualFactory.Instance.UsePreCompiledShader();

 <param name="option">The map contains the shader option for precompiling.</param>
 <return>True if the pre-compiled shader is added, otherwise false.</return>

Sample 까지 API에 첨부해주셔서 잘 이해가 됩니다 👍
큰 구현이었는데 수고많으셨습니다!

Copy link
Contributor

@Seoyeon2Kim Seoyeon2Kim left a comment

Choose a reason for hiding this comment

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

사용 example과 자세한 description덕분에 사용성이 증가했네요 LGTM

@TizenAPI-Bot
Copy link
Collaborator

Internal API Changed

Added: 1, Removed: 0, Changed: 0

Added

+ /// <since_tizen>none</since_tizen
+ [EditorBrowsable(EditorBrowsableState.Never)]
+ System.Boolean Tizen.NUI.VisualFactory::AddPrecompileShader(Tizen.NUI.PropertyMap)

@tscholb
Copy link
Contributor Author

tscholb commented Sep 13, 2024

@taehyub, @Seoyeon2Kim , @hinohie
빠르고 친절한 리뷰 감사드립니다 👍
반영하겠습니다

@tscholb tscholb merged commit 09e3c21 into Samsung:DevelNUI Sep 13, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants