-
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
Stickers #1958
base: master
Are you sure you want to change the base?
Stickers #1958
Conversation
Use an array of floats for key data instead of separate vectors and quaternions. Update importer to use new APIs.
We are using horizontal/ vertical flip shaders, or post effects just for inverting a scene. we should be modifying mvp instead to rotate or flip the scene instead of all that overhead which is not efficient. Which will also avoid creating unnecessary render targets. |
Couple of tests I believe would be useful. |
File sdcard = Environment.getExternalStorageDirectory(); | ||
mDirectory = sdcard.getAbsolutePath() + "/GearVRFScreenshots/"; | ||
File d = new File(mDirectory); | ||
d.mkdirs(); |
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.
Probably the fact that WRITE_EXTERNAL_STORAGE is required should be mentioned. Also I'd recommend checking the return value of mkdirs and throwing an exception immediately if needed.
Have you verified screenshotting with all backend adapters? |
@@ -236,6 +233,7 @@ void onDrawEye(int eye, int swapChainIndex, boolean use_multiview) { | |||
|
|||
renderTarget.cullFromCamera(mMainScene, mainCameraRig.getCenterCamera(), mRenderBundle.getShaderManager()); | |||
captureCenterEye(renderTarget, false); | |||
captureSticker(); |
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.
What about other backend adapters?
What about generating stickers in "headless mode"? Where the framework is only used to render into the images as a background "service"? Should there be all new backend adapter for that mode? |
|
||
GLuint* &pbos = gRenderer->getPBOs(); | ||
if(pbos == NULL){ | ||
pbos = new GLuint[3]; |
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.
Please use a constant for the number of pbos.
@@ -202,10 +202,13 @@ class Renderer { | |||
} | |||
return nullptr; | |||
} | |||
virtual bool readRenderResultInPBO(int) = 0; |
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.
Not a big deal but PBO is not directly applicable to Vulkan, correct? A more generic name like buffer may be better.
Looks pretty good overall. |
Needs rebase. Are any of the comments valid? |
Please generalize beyond stickers. This should be a component you attach to the Scene object that owns the render target to capture textures. |
@thomasflynn @NolaDonato In the current approach there is a way to set the interval at which the sticker(screenshot) needs to be captured. So if we move to RenderTarget based approach, onDrawFrame will be called every time. That means no interval but capturing every frame. Do we want interval for RenderTarget approach as well? |
Yes that would be useful. We should be able to specify it in terms of frames or seconds |
Should we close? |
We still want the feature but it can go into SXR instead of GVRF |
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.
Android system webviwver
Added screenshot capture feature by using 3 Pixel Buffer objects.
New class introduced : GVRSticker(GVRContext gvrContext, String tag, long interval)
Methods:
GearVRf DCO signed off by: Abhijit Jagdale [email protected]