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

Changed the default mode for the PlaneRenderer. #39

Merged
merged 1 commit into from Mar 16, 2022

Conversation

RGregat
Copy link
Contributor

@RGregat RGregat commented Mar 12, 2022

PlaneRenderMode.RENDER_ALL was set as the default mode but the doc states clearly that this mode is very expensive. A short check on the Profile verified this statement with 500ms spikes. The default mode is now set to PlaneRenderMode.RENDER_TOP_MOST.

…ates clearly that this mode is very expensive.
@ThomasGorisse
Copy link
Contributor

ThomasGorisse commented Mar 13, 2022

Great found @RGregat !!!
I couldn't figure out why was the last Model Viewer laggy after some planes detected.
We definitely have to move the PlaneVisualizer to kotlin. It can easily be a PlaneNode and ShadowPlaneNode.
@RGregat Would you mind doing it just like in the answer you made there #30 (comment) ?
I think that because glTF is at the end not so consuming comparing to programmatic mesh, maybe it could be a lot cleaner and customizable for users to use a Plane.glb and ShadowPlane.glb choice here.
I don't know if it's an issue with ARCore polygons @grassydragon ?

@grassydragon
Copy link
Contributor

It can easily be a PlaneNode and ShadowPlaneNode.

Won't adding and removing many Nodes create a lot of objects for garbage collection?

I don't know if it's an issue with ARCore polygons @grassydragon?

I think we can only generate the polygons at runtime.

@grassydragon
Copy link
Contributor

Is the topMostPlane actually the top most here?

arFrame.updatedPlanes.firstOrNull()?.let { topMostPlane ->

@grassydragon
Copy link
Contributor

Before moving the PlaneVisualizer to Kotlin let's decide what to do with the Renderable and RenderableInstance classes: #40

@ThomasGorisse
Copy link
Contributor

Is the topMostPlane actually the top most here?

arFrame.updatedPlanes.firstOrNull()?.let { topMostPlane ->

Yes it should
There is definetly something to do to clean this part:

if (isVisible) {
// If we hit a plane, return the hit point.
val hitResult = arFrame.hitTest(
xPx = session.displayWidth / 2.0f,
yPx = session.displayHeight / 2.0f,
plane = true,
depth = false,
instantPlacement = false
)
// Calculate the focusPoint. It is used to determine the position of
// the visualized grid.
val focusPoint = getFocusPoint(arFrame.frame, hitResult)
materialInstance?.setParameter(
MATERIAL_SPOTLIGHT_FOCUS_POINT,
Position(focusPoint.x, focusPoint.y, focusPoint.z)
)
}
if (planeRendererMode == PlaneRendererMode.RENDER_ALL) {
renderAll(session.allPlanes)
} else if (planeRendererMode == PlaneRendererMode.RENDER_TOP_MOST) {
arFrame.updatedPlanes.firstOrNull()?.let { topMostPlane ->
renderPlane(topMostPlane)
}
}
// Check for not tracking Plane-Trackables and remove them.
cleanupOldPlaneVisualizer()

@ThomasGorisse
Copy link
Contributor

It can easily be a PlaneNode and ShadowPlaneNode.

Won't adding and removing many Nodes create a lot of objects for garbage collection?

You are probably right even if it would bring some great Node features we can use directly here.

I don't know if it's an issue with ARCore polygons @grassydragon?

I think we can only generate the polygons at runtime.

Would you mind doing the move from RenderableDefinittion to Filament direct mesh creation?
I definetly don't feel confortable with the mesh creation.

@ThomasGorisse
Copy link
Contributor

Before moving the PlaneVisualizer to Kotlin let's decide what to do with the Renderable and RenderableInstance classes: #40

Big subject here even if I'm conviced that most of the Renderable and RenderableInstance with the old sfb managed can be cleaned quite quickly. Resulting in a quite simple class.

@grassydragon
Copy link
Contributor

grassydragon commented Mar 13, 2022

Would you mind doing the move from RenderableDefinittion to Filament direct mesh creation?

We just need to make sure that we aren't missing any important features that Sceneform provided for mesh creation. I actually only used the direct Filament approach.

@RGregat
Copy link
Contributor Author

RGregat commented Mar 15, 2022

Is this an example for the filament way to create a mesh? https://github.com/google/filament/blob/main/samples/hellotriangle.cpp

@ThomasGorisse
Copy link
Contributor

ThomasGorisse commented Mar 15, 2022

Is this an example for the filament way to create a mesh? https://github.com/google/filament/blob/main/samples/hellotriangle.cpp

This is the C++ version.
I created an Issue for each points of this discussion.
Answer here: #43

@ThomasGorisse ThomasGorisse merged commit c0c0659 into main Mar 16, 2022
@ThomasGorisse ThomasGorisse mentioned this pull request Mar 16, 2022
@grassydragon grassydragon deleted the hotfix/changed_default_plane_renderer_mode branch March 30, 2022 19:13
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