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

fix(Visual): add SRP support on CameraColorOverlay #477

Closed

Conversation

Wolar
Copy link

@Wolar Wolar commented Dec 8, 2019

Probably related to https://github.com/ExtendRealityLtd/VRTK/issues/1959 so I didn't open a new issue.

Copy link
Contributor

@bddckr bddckr left a comment

Choose a reason for hiding this comment

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

I didn't test the actual functionality, so code review only.

Runtime/Visual/CameraColorOverlay.cs Outdated Show resolved Hide resolved
@Wolar
Copy link
Author

Wolar commented Dec 9, 2019

No problem with that change, should be changed now

@bddckr
Copy link
Contributor

bddckr commented Dec 9, 2019

We're friends of the idea of "once a PR is merged the commits it brought should be primarily helpful for the readers of the commits". As such we'd like you to squash the commits before we merge (whether we merge is pending someone else to review). So just squash and force-push! (GitHub thankfully has good UX for reviewing force-pushes even.)

@Wolar Wolar force-pushed the fix/camera-color-overlay-srp branch from 0aac7d2 to 76109f9 Compare December 9, 2019 16:41
@Wolar
Copy link
Author

Wolar commented Dec 9, 2019

Ok, no problem, done

@Wolar
Copy link
Author

Wolar commented Dec 9, 2019

I just found out that this solution is not enough for Quest. For some reason I'm getting

OPENGL NATIVE PLUG-IN ERROR: GL_INVALID_OPERATION: Operation illegal in current state
(Filename: ./Runtime/GfxDevice/opengles/GfxDeviceGLES.cpp Line: 353)

I've tried to replace that GL rendering code in PostRender with something like code bellow for SRP
var cmd = CommandBufferPool.Get("FadeScreenPass"); cmd.SetViewProjectionMatrices(Matrix4x4.identity, Matrix4x4.identity); cmd.DrawMesh(RenderingUtils.fullscreenMesh, Matrix4x4.identity, workingMaterial); cmd.SetViewProjectionMatrices(sceneCamera.worldToCameraMatrix, sceneCamera.projectionMatrix); context.ExecuteCommandBuffer(cmd); CommandBufferPool.Release(cmd); context.Submit();

but it didn't help and I can't figure out how to fix this / fixing this is beyond my skills.

@thestonefox
Copy link
Member

This should be fixed now

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.

None yet

3 participants