LBRYCommunity/lbry-desktop

Fork

You've already forked lbry-desktop

Code Issues 596 Pull requests 6 Projects Releases 238 Packages Wiki Activity

Add 3D-file-viewer component #1558

Merged

btzr-io merged 8 commits from three-viewer into master

2018-07-19 16:17:06 +02:00

Conversation 20 Commits 8 Files changed 11 +423 -1

btzr-io commented

2018-06-06 08:11:58 +02:00

(Migrated from github.com)

Copy link

Changes

A basic ThreeJS 3D viewer based on Autodesk/3DViewerComponent

Add 3D file-viewing to LBRY desktop app https://github.com/lbryio/lbry-app/issues/1409

Test

lbry://stl#b4f0c103be510a516c8ebb98ceb093dc745e7981

Todo (WIP)

Threejs + React.
Load blob from claim
Show Grid
Render 3D-model.
Add 3D-file support for getMediaType function https://github.com/lbryio/lbry-redux/pull/41
Investigate render bug. https://github.com/mrdoob/three.js/issues/12499#issuecomment-339780204
Ensure that STL files have valid normal data.
Show loading message
Handle Errors
Adaptive Theme (Dark / Light) 🎨
STL loader support.
OBJ loader support.

Preview

### Changes A basic ThreeJS 3D viewer based on [Autodesk/3DViewerComponent ](https://github.com/Autodesk/3DViewerComponent) - Add 3D file-viewing to LBRY desktop app https://github.com/lbryio/lbry-app/issues/1409 ### Test > lbry://stl#b4f0c103be510a516c8ebb98ceb093dc745e7981 ### Todo (WIP) - [x] Threejs + React. - [x] Load blob from claim - [x] Show Grid - [x] Render 3D-model. - [x] Add 3D-file support for getMediaType function https://github.com/lbryio/lbry-redux/pull/41 - [X] Investigate render bug. https://github.com/mrdoob/three.js/issues/12499#issuecomment-339780204 - [x] Ensure that STL files have valid normal data. - [x] Show loading message - [x] Handle Errors - [x] Adaptive Theme (Dark / Light) :art: - [x] STL loader support. - [ ] OBJ loader support. ### Preview <img src=https://user-images.githubusercontent.com/14793624/41011388-ece56a30-68f9-11e8-94ae-670a364cb0f9.gif width=350> <img src=https://user-images.githubusercontent.com/14793624/41011434-2613bbfe-68fa-11e8-9d4c-d2854e40f801.gif width=350>

skhameneh (Migrated from github.com) reviewed 2018-06-06 08:11:58 +02:00

neb-b commented

2018-06-08 22:03:01 +02:00

(Migrated from github.com)

Copy link

@btzr-io Is this ready for review?

👍 1

btzr-io commented

2018-06-09 22:28:03 +02:00

(Migrated from github.com)

Copy link

@seanyesmunt Maybe we should use a progress bar instead of the current spinner for the loading screen, What do you think?

@seanyesmunt Maybe we should use a progress bar instead of the current spinner for the loading screen, What do you think? ![](http://uxmovement.com/wp-content/uploads/2016/11/spinner-bar-4seconds.png)

btzr-io commented

2018-06-09 22:37:14 +02:00

(Migrated from github.com)

Copy link

@seanyesmunt So I think it's ready for review.

neb-b commented

2018-06-12 18:34:23 +02:00

(Migrated from github.com)

Copy link

Will check this out today!

👍 1

btzr-io commented

2018-06-12 19:15:53 +02:00

(Migrated from github.com)

Copy link

ok, I'll setup a test branch...

btzr-io commented

2018-06-13 06:48:14 +02:00

(Migrated from github.com)

Copy link

@seanyesmunt Test branch: https://github.com/lbryio/lbry-app/tree/test-3d

neb-b commented

2018-06-13 18:15:22 +02:00

(Migrated from github.com)

Copy link

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.

👍 2

neb-b commented

2018-06-27 08:14:14 +02:00

(Migrated from github.com)

Copy link

Ok! Ready to get this in. I'm seeing Sorry, looks like we can't play this file. when I go to the file in your description. Not sure if this is from a recent change or your code.

Is this dependent on https://github.com/lbryio/lbry-app/pull/1576?

Ok! Ready to get this in. I'm seeing `Sorry, looks like we can't play this file.` when I go to the file in your description. Not sure if this is from a recent change or your code. Is this dependent on https://github.com/lbryio/lbry-app/pull/1576?

👍 1

neb-b commented

2018-06-27 08:15:33 +02:00

(Migrated from github.com)

Copy link

Oh I see that PR references this component, so it's the other way around. Ignore me

👍 1

btzr-io commented

2018-06-28 04:12:20 +02:00

(Migrated from github.com)

Copy link

@seanyesmunt where did you test it? check the test-branch: https://github.com/lbryio/lbry-app/pull/1558#issuecomment-396812301
This is dependent of #1576 so you should probably review that one first ✌️

@seanyesmunt where did you test it? check the `test-branch`: https://github.com/lbryio/lbry-app/pull/1558#issuecomment-396812301 This is dependent of #1576 so you should probably review that one first :v:

tzarebczan commented

2018-07-06 16:31:01 +02:00

(Migrated from github.com)

Copy link

@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.

👍 1

btzr-io commented

2018-07-08 22:38:49 +02:00

(Migrated from github.com)

Copy link

@tzarebczan fixed in 5e1d6c2901

@tzarebczan fixed in https://github.com/lbryio/lbry-app/pull/1576/commits/5e1d6c290157a8d245139835e3677c4952c19c4a

tzarebczan commented

2018-07-12 16:39:03 +02:00

(Migrated from github.com)

Copy link

@btzr-io tried an stl file yesterday on this branch and got "this file cannot be played".

btzr-io commented

2018-07-13 05:08:08 +02:00

(Migrated from github.com)

Copy link

@tzarebczan There is no implementation of the component yet 🙃

@tzarebczan There is no implementation of the component yet :upside_down_face:

neb-b commented

2018-07-16 16:00:59 +02:00

(Migrated from github.com)

Copy link

Planning to merge this sometime this soon.

neb-b (Migrated from github.com) reviewed 2018-07-16 16:11:19 +02:00

src/renderer/component/threeViewer/index.jsx

neb-b (Migrated from github.com) commented

2018-07-16 16:11:18 +02:00

Copy link

You shouldn't need to bind this in these functions. handleStart (and the others) uses the same this reference

You shouldn't need to bind `this` in these functions. `handleStart` (and the others) uses the same `this` reference

neb-b (Migrated from github.com) requested changes 2018-07-16 16:19:42 +02:00

neb-b (Migrated from github.com) left a comment

Copy link

Very small comments. Also asking @skhameneh for a review. I will continue to test.

src/renderer/component/threeViewer/index.jsx

neb-b (Migrated from github.com) commented

2018-07-16 16:18:25 +02:00

Copy link

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.

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.

src/renderer/component/threeViewer/internal/loader.js

neb-b (Migrated from github.com) commented

2018-07-16 16:18:57 +02:00

Copy link

I don't think this function is being used? I see it being called below, but that value isn't being used.

neb-b (Migrated from github.com) reviewed 2018-07-16 16:39:52 +02:00

src/renderer/component/fileRender/view.jsx

neb-b (Migrated from github.com) commented

2018-07-16 16:39:51 +02:00

Copy link

I added this so it works, this component should probably live inside of component/viewers/... to keep it consistent.

I added this so it works, this component should probably live inside of `component/viewers/...` to keep it consistent.

neb-b commented

2018-07-16 16:40:47 +02:00

(Migrated from github.com)

Copy link

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.

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.

skhameneh commented

2018-07-16 16:41:16 +02:00

(Migrated from github.com)

Copy link

Hey @btzr-io can you add debug info for geometry count?
@seanyesmunt is having trouble when working with a demo, his machine freezes when leaving the viewer.

I'd also like to note that it might be good to explore simplifying complex geometries before rendering:
https://threejs.org/examples/?q=simplifi#webgl_modifier_simplifier

Hey @btzr-io can you add debug info for geometry count? @seanyesmunt is having trouble when working with a demo, his machine freezes when leaving the viewer. I'd also like to note that it might be good to explore simplifying complex geometries before rendering: https://threejs.org/examples/?q=simplifi#webgl_modifier_simplifier

👍 1

neb-b commented

2018-07-16 16:43:20 +02:00

(Migrated from github.com)

Copy link

Possibly realated to running in dev mode. I shared the binaries internally and didn't hear any issues. I'll try running those.

neb-b commented

2018-07-16 20:02:55 +02:00

(Migrated from github.com)

Copy link

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.

👍 1

btzr-io (Migrated from github.com) reviewed 2018-07-19 00:53:38 +02:00

src/renderer/component/threeViewer/internal/loader.js

btzr-io (Migrated from github.com) commented

2018-07-19 00:53:38 +02:00

Copy link

34ba2cf3eb/src/renderer/component/threeViewer/internal/loader.js (L24)

https://github.com/lbryio/lbry-desktop/blob/34ba2cf3ebad8383e045d606ff8b5c27921e53a7/src/renderer/component/threeViewer/internal/loader.js#L24

btzr-io commented

2018-07-19 09:15:13 +02:00

(Migrated from github.com)

Copy link

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).

Not sure what you mean by after, the only time I'm seeing this issue is while loading the model:

Loading 3D model...

Looks like the loader is blocking the UI until the loading and parsing process of the file is completed
https://stackoverflow.com/questions/48636384/web-workers-for-loading-object-in-three-js

Maybe we can simplify the geometry in dev mode?

I tried to follow the webgl_modifier_simplifier example but the model disappears from the scene.

>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). Not sure what you mean by after, the only time I'm seeing this issue is while loading the model: > Loading 3D model... Looks like the loader is blocking the UI until the loading and parsing process of the file is completed https://stackoverflow.com/questions/48636384/web-workers-for-loading-object-in-three-js > Maybe we can simplify the geometry in dev mode? I tried to follow the `webgl_modifier_simplifier` example but the model disappears from the scene.

neb-b (Migrated from github.com) approved these changes 2018-07-19 15:49:30 +02:00

Reviewers

No reviewers

skhameneh

neb-b

Labels

Clear labels

Work is part of a proposal

Beamer is waiting on you!

Discuss this issue at the next planning meeting, then remove this label

Welcome to Hacktoberfest

No knowledge of the existing code required

level: 2

Some knowledge of the existing code is recommended

level: 3

Significant knowledge of the existing code is recommended

level: 4

Intimate knowledge of the existing code is recommended

merge when green

needs: exploration

Solution unclear, needs research

needs: grooming

Issue needs to be groomed before work can start

needs: priority

Priority level needs to be defined

needs: repro

Issue needs step-by-step instructions on how to reproduce in latest code

needs: tech design

Needs technical design signoff before implementation can begin

Issue is blocking release, do ASAP

priority: high

Work needs to be moved into sprint ASAP

priority: low

Work should be done but can stay in the backlog for now

priority: medium

Work needs to be done within 2-3 sprints

protocol dependent

recsys

redesign

general redesign not prioritize for anyone release

regression

resilience

sdk dependent

Requires work on lbry-sdk repo

Tom's Wishlist

trending

type: bug

Existing functionality is wrong or broken

type: discussion

A conversation. No specific changes requested

type: improvement

Existing (or partially existing) functionality needs to be changed

type: new feature

New functionality that does not exist yet

type: refactor

Minimal user-visible changes, but significant internal work

type: task

Either work that's not related to the code, or a small chore that does not fit into other categories

type: testing

Solution needs additional user testing

unplanned

Work that was not planned into the spirnt, took priority over planned work

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbry-desktop#1558

Reference in a new issue

Repository

LBRYCommunity/lbry-desktop

Title

Body

No description provided.

Delete branch "three-viewer"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?