Add 3D-file-viewer component #1558
Conversation
|
@btzr-io Is this ready for review? |
|
@seanyesmunt Maybe we should use a progress bar instead of the current spinner for the loading screen, What do you think? |
|
@seanyesmunt So I think it's ready for review. |
|
Will check this out today! |
|
ok, I'll setup a test branch... |
|
@seanyesmunt Test branch: https://github.com/lbryio/lbry-app/tree/test-3d |
|
Hey @btzr-io. Probably won't get a chance to look at this until the redesign is live. This will be one of the first things that go in after that though. |
|
Ok! Ready to get this in. I'm seeing Is this dependent on #1576? |
|
Oh I see that PR references this component, so it's the other way around. Ignore me |
|
@seanyesmunt where did you test it? check the |
|
@btzr-io can you try lbry://stl? Not seeing anything but the grid when it first opens. Went out and back into the claim, then it worked. |
|
@tzarebczan fixed in 5e1d6c2 |
|
@btzr-io tried an stl file yesterday on this branch and got "this file cannot be played". |
|
@tzarebczan There is no implementation of the component yet 🙃 |
|
Planning to merge this sometime this soon. |
| onStart: this.handleStart(this), | ||
| onLoad: this.handleReady.bind(this), | ||
| onError: this.handleError.bind(this), | ||
| onProgress: this.handleProgress.bind(this), |
There was a problem hiding this comment.
You shouldn't need to bind this in these functions. handleStart (and the others) uses the same this reference
There was a problem hiding this comment.
Very small comments. Also asking @skhameneh for a review. I will continue to test.
|
|
||
| handleProgress() { | ||
| // const progress = (currentItem / totalItems) * 100; | ||
| // console.info(currentItem, totalItems, progress); |
There was a problem hiding this comment.
I think we will hold off on the progress bar for now, we have a progress percentage below the file.
I am going to add analytics for start to stream time, that will give us an idea of average time spent looking at the loading screen. If it's decently high I think we can add it.
| manager.onError = onError; | ||
|
|
||
| return manager; | ||
| }; |
There was a problem hiding this comment.
I don't think this function is being used? I see it being called below, but that value isn't being used.
There was a problem hiding this comment.
| @@ -2,6 +2,7 @@ | |||
| import React from 'react'; | |||
| import LoadingScreen from 'component/common/loading-screen'; | |||
| import PdfViewer from 'component/viewers/pdfViewer'; | |||
| import ThreeViewer from 'component/threeViewer'; | |||
There was a problem hiding this comment.
I added this so it works, this component should probably live inside of component/viewers/... to keep it consistent.
|
We are seeing some issues where the app becomes super un-responsive after viewing 3d files (I've had to restart twice after my computer freezes). Will continue to test, but it should be simple stuff to help improve performance. |
|
Hey @btzr-io can you add debug info for geometry count? I'd also like to note that it might be good to explore simplifying complex geometries before rendering: |
|
Possibly realated to running in dev mode. I shared the binaries internally and didn't hear any issues. I'll try running those. |
|
Seems to work fine with a binary. I'm guessing it was just from running in dev mode. Maybe we can simplify the geometry in dev mode? That should help. |
Not sure what you mean by after, the only time I'm seeing this issue is while loading the model:
Looks like the loader is blocking the UI until the loading and parsing process of the file is completed
I tried to follow the |
Changes
A basic ThreeJS 3D viewer based on Autodesk/3DViewerComponent
Test
Todo (WIP)
Preview