-
Notifications
You must be signed in to change notification settings - Fork 217
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
Allow specifying texture parameters. #87
Comments
We'll take this one up as it is important and requires immediate attention. |
Hi @thomasflynn , We have done some implementation as a test for implementing texture enhancement parameters. The change commit is here(https://github.com/SRIB-GRAPHICS/GearVRf/commit/8f0d6d66add2407247551913dd316ccc84661fc4) on the branch (https://github.com/SRIB-GRAPHICS/GearVRf/tree/texture-enhancement). Please review the implementation approach so that we can proceed on similar lines. Referencing other members for reviews. [ @soumyajitdeb @jason2kim ] |
did a quick skim of the code. some comments:
|
Hi @thomasflynn , We've taken care of the point (2) [https://github.com/SRIB-GRAPHICS/GearVRf/commit/1d8bf33e11360067abcf2eddf49cd47513886caa] & (3) [https://github.com/SRIB-GRAPHICS/GearVRf/commit/b8ac24e69008773f75ed1d2e5c69848f1bbec224] in the latest commit. For point (1), we think that using enum along with switch is much cleaner as we tried using String for the first time and the code was not so clean once we were trying to pass values to JNI as there we will have to compare in the similar fashion. if you can please suggest some better ways than we will be happy to work on that. These commits are just for getting the feel of how to proceed, once we are clear, we'll clean the code as per our regualr patters. Thanks. |
I did a quick skim and in general I think it's good. I just have a few points:
One possible way of doing this is to declare enums in Texture class that maps directly to gl constants. Like follows:
And now your function can be:
This is good for some reasons. First, it avoid the switch and the calling code doesn't need to include GL headers. Then it makes less error prone (client code won't be able to pass GL_LINEAR_MIPMAP_LINEAR to mag filter by mistake or mess up min/mag filter parameter order. Lastly, the cast from the enum to int will be safe since you declared enum as
|
Hi @phi-lira , Thank you for such an elaborate explanation. We will surely start with this new suggestion. Thanks |
Hi, We tried the suggested pattern on the Java side and, with some modifications we came to this [https://github.com/SRIB-GRAPHICS/GearVRf/commit/0337010fd7bb54296d3f13eb2aa4b2cf03a652d8] state. Please give your input. The conversion of float to int is still not taken care of. We are working on that. Thanks. |
Need to allow setting of texture parameters: min & mag filters, format, type, aniso, etc.
The text was updated successfully, but these errors were encountered: