-
Notifications
You must be signed in to change notification settings - Fork 81
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
[HDR & WCG patch1] Fix videolayer render issues #663
base: hdr_invest
Are you sure you want to change the base?
Conversation
In Linux env, the jsontestlayer expects videolayer renderer calls which wasn't built. Change-Id: Iab15dddcaf1214e7ec8ee40485a719328f98a17f Tests: In Linux environment, testlayers works for bgra. Tracked-On: None Signed-off-by: Kishore Kadiyala <kishore.kadiyala@intel.com>
@@ -46,6 +46,7 @@ testlayers_SOURCES = \ | |||
./common/layerrenderer.cpp \ | |||
./common/gllayerrenderer.cpp \ | |||
./common/glcubelayerrenderer.cpp \ | |||
./common/videolayerrenderer.cpp \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change supposed to be part of another pull request? Suggest combining it with the corresponding commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, there are a couple of patches need to be reviewed in parallel and there are some commit order dependence among them. The list of PRs for these patches have been committed into my own git repo:
https://github.com/shuangwan01/IA-Hardware-Composer/pulls
Not sure if there is an efficient way to push PRs into this project directly and manage the further revisions accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it better to combine the patches under one commit? The specific commit order requirement may be difficult to maintain because only the author and reviewers are familiar with it right? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, agree with you that one commit will make the PR maintenance more easier. But this feature is involving 3 authors and feature itself even doesn't have tested under Android. So it's expected large number of revisions according to real tests/debugs and review comments. Breaking down into small code pieces will make this practice more efficient and is also easier for community to conduct code review :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds appropriate :) 👍
In Linux env, the jsontestlayer expects videolayer renderer calls
which wasn't built.
Change-Id: Iab15dddcaf1214e7ec8ee40485a719328f98a17f
Tests: In Linux environment, testlayers works for bgra.
Tracked-On: None
Signed-off-by: Kishore Kadiyala kishore.kadiyala@intel.com