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

Flickering of glTF model #139

Closed
brettkromkamp opened this issue Nov 22, 2020 · 34 comments
Closed

Flickering of glTF model #139

brettkromkamp opened this issue Nov 22, 2020 · 34 comments
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed 🚀 enhancement New feature or request

Comments

@brettkromkamp
Copy link

Hi,

I've been experimenting with displaying glTF models and re-used the glTF example (https://github.com/webarkit/ARnft/blob/master/examples/arNFT_gltf_example.html) to do so. Nonetheless, when rendering the model there is extreme flickering. I have attached two screenshots to try to exemplify the issue. The first screenshot below shows the model using an online glTF model viewer (https://gltf-viewer.donmccurdy.com/).

model-gltf-viewer

The second screenshot is of the actual AR rendering of the model. The model is actively flickering and the overall quality is just very poor.

gltf-render-arntf

So, my question is: am I doing something obviously wrong? As far as I can tell, the model I have used should be suitable for AR purposes (I have used it with AR.js without these kind of issues).

@brettkromkamp
Copy link
Author

If necessary, I can provide the (binary) glTF file if that makes trouble-shooting this issue, easier.

@brettkromkamp
Copy link
Author

brettkromkamp commented Nov 22, 2020

It looks like there is a problem with the normals of the model. I don't believe there is, though.

gltf-render-arntf2

@kalwalt
Copy link
Member

kalwalt commented Nov 22, 2020

Hi @brettkromkamp welcome to WeBARKit and ARnft, and thank you for testing! I would test your model, maybe it can help set logarithmicDepthBuffer to false but you need to add in the ThreejsRenderer constructor a new entry:
logarithmicDepthBuffer: false
Sorry you can not add custom options to the renderer at the moment...

@kalwalt kalwalt self-assigned this Nov 22, 2020
@kalwalt kalwalt added bug Something isn't working 🚀 enhancement New feature or request help wanted Extra attention is needed labels Nov 22, 2020
@kalwalt
Copy link
Member

kalwalt commented Nov 22, 2020

...continuing. You need also to rebuild the project with webpack: npm run build-es6 for a production build or npm run dev-es6 for a development build.

If necessary, I can provide the (binary) glTF file if that makes trouble-shooting this issue, easier.

Yes it would be nice, thank you.

@brettkromkamp
Copy link
Author

@kalwalt Hi! Thanks for a reply on a Sunday 👍 I've uploaded the model (zipped because GitHub doesn't allow uploading .glb files directly). I'll take a look at logarithmicDepthBuffer: false as suggested and let you know how it goes.

brave-robot.zip

@brettkromkamp
Copy link
Author

@kalwalt Not sure I added logarithmicDepthBuffer: false to the right place (in the ThreejsRenderer.js file):

constructor (configData, canvasDraw, root) {
    this.root = root
    this.renderer = new THREE.WebGLRenderer({
      canvas: canvasDraw,
      alpha: configData.renderer.alpha,
      antialias: configData.renderer.antialias,
      precision: configData.renderer.precision,
      logarithmicDepthBuffer: false
    })
    this.renderer.setPixelRatio(window.devicePixelRatio)
    this.scene = new THREE.Scene()
    this.camera = new THREE.Camera()
}

It made no difference. I'll keep on digging around :)

@kalwalt
Copy link
Member

kalwalt commented Nov 22, 2020

@kalwalt Hi! Thanks for a reply on a Sunday +1 I've uploaded the model (zipped because GitHub doesn't allow uploading .glb > files directly). I'll take a look at logarithmicDepthBuffer: false as suggested and let you know how it goes.

No problem, we are in soft lock down here... 🙂

@kalwalt Not sure I added logarithmicDepthBuffer: false to the right place (in the ThreejsRenderer.js file):

constructor (configData, canvasDraw, root) {
    this.root = root
    this.renderer = new THREE.WebGLRenderer({
      canvas: canvasDraw,
      alpha: configData.renderer.alpha,
      antialias: configData.renderer.antialias,
      precision: configData.renderer.precision,
      logarithmicDepthBuffer: false
    })
    this.renderer.setPixelRatio(window.devicePixelRatio)
    this.scene = new THREE.Scene()
    this.camera = new THREE.Camera()
}

It made no difference. I'll keep on digging around :)

did you rebuild the project as i explained?

@brettkromkamp
Copy link
Author

brettkromkamp commented Nov 22, 2020

@kalwalt Yes, I did a development re-build (with npm run dev-es6). Suppose it doesn't make a difference with a production build, but I will try.

I’ll also try with different models.

@kalwalt
Copy link
Member

kalwalt commented Nov 22, 2020

@kalwalt Yes, I did a development re-build (with npm run dev-es6). Suppose it doesn't make a difference with a production build, but I will try.

I’ll also try with different models.

It shouldn't be a difference, the real difference should be in the size of the built file.
In theory because ARnft is based on Three.js it should be possible to view the model as in the glTF Viewer. Probably it is needed some extra settings to the renderer.

@brettkromkamp
Copy link
Author

@kalwalt
Copy link
Member

kalwalt commented Nov 24, 2020

I think i need to add an option to inject additional settings to the ThreejsRenderer class, at the moment it is very poor! This is on my radar and to do list!

@brettkromkamp
Copy link
Author

That's my thinking, as well. I will try to set some time aside this weekend to work on the issue which will hopefully result in the accompanying PR. I'm also thinking that (hopefully) the glTF Viewer can provide some guidance on how to go about this: https://github.com/donmccurdy/three-gltf-viewer/blob/master/src/viewer.js

@kalwalt
Copy link
Member

kalwalt commented Nov 24, 2020

Thank you @brettkromkamp a PR is welcome!

@kalwalt
Copy link
Member

kalwalt commented Nov 30, 2020

@brettkromkamp For sure i will add other parameters settings in the ThreejsRenderer constructor, but Three.js renderer have a lot of properties and methods: I think that we need to implement a method to retrieve the renderer itself and give us more freedom.

@brettkromkamp
Copy link
Author

@kalwalt I haven't got too far with this issue. Perhaps it's better if you take this issue and I'll look around for (or, you suggest) other issues that I can work on. What do you think?

@kalwalt
Copy link
Member

kalwalt commented Dec 1, 2020

@brettkromkamp yes, i will take this issue. Regarding to issues in ARnft, there are different topics,for example I'm working to add support for Aframe ( see PR #110) and Babylon.js (this branch https://github.com/webarkit/ARnft/tree/babylonjs-renderer), we should also improve the Filtering #3; another issue is to try to add new useful methods to ARnft #5. I would also know your opinion about how improve it and if you have other ideas. Feel free to choose your topic. We are open to collaboration and to welcome new members to the webarkit team. 🙂

@kalwalt
Copy link
Member

kalwalt commented Dec 1, 2020

@brettkromkamp PR #143 is on the way!

@brettkromkamp
Copy link
Author

@kalwalt Superficial look... but, that seems like it is going in the right direction :) I'll gladly test.

@kalwalt
Copy link
Member

kalwalt commented Dec 1, 2020

Wait for the next commits in the PR!

@brettkromkamp
Copy link
Author

brettkromkamp commented Dec 6, 2020

@kalwalt Do you need any testing of the feature-extends-threejsrenderer branch before merging? Or, good to go?

@kalwalt
Copy link
Member

kalwalt commented Dec 6, 2020

Yes, It would be nice some testing, I think i need only to do some corrections/cleaning, you can find my comments in the PR #143

@kalwalt
Copy link
Member

kalwalt commented Dec 6, 2020

@brettkromkamp i merged #143 in the dev branch. If you find some bugs or issues post here, we will fix them.

@brettkromkamp
Copy link
Author

@kalwalt Sorry for the delay. Will do!

@kalwalt
Copy link
Member

kalwalt commented Dec 8, 2020

@brettkromkamp i should also add your credits for the model, i will do when i have a bit of time. 🙂

@brettkromkamp
Copy link
Author

@brettkromkamp i should also add your credits for the model, i will do when i have a bit of time. 🙂

@kalwalt Thanks. Much appreciated.

@kalwalt
Copy link
Member

kalwalt commented Dec 10, 2020

@brettkromkamp tested the dev branch on Mobile device. The model looks much, much better than the initial case. I think i will merge it. Model appears shifted (not centered on the pinball image), don't know if it is caused by the new changes.

@kalwalt
Copy link
Member

kalwalt commented Dec 10, 2020

See this screenshot:
Screenshot_20201210-220149

@brettkromkamp
Copy link
Author

@kalwalt Model looks much better (in the above photo). I will test against the development branch, as soon as possible. The model could be offset... perhaps the origin has changed? I'll check.

@kalwalt
Copy link
Member

kalwalt commented Dec 10, 2020

@brettkromkamp thank you! No, i tested now also the simple gltf example with the duck model and it is affected by the same issue. I think it is not caused by the new code. I will open a new issue to track this last problem.

@brettkromkamp
Copy link
Author

brettkromkamp commented Dec 10, 2020

@kalwalt Ok. Anyway, testing of rendering of the Brave Robot model is looking much better.

arnft-brave-robot-test.

We can probably close this issue then, or? :)

@kalwalt
Copy link
Member

kalwalt commented Dec 10, 2020

Well, i think it is a big difference! .. and it looks better on your picture! I will close this isssue when i will merge it in master. Thank you again! 😄

@kalwalt
Copy link
Member

kalwalt commented Dec 14, 2020

Now the new features are in master branch!

@kalwalt kalwalt closed this as completed Dec 14, 2020
@jalamprea
Copy link

I know this issue is closed, but did you know the issue is still presenting? the flickering on gltf models is appearing even on the basic sample with the Duck model.
On my end I had to replace the pure Three Camera for a Perspective camera, adjust the fov to 40, and use the own camera.updateProjectionMatrix() method instead of set the matrix from the artoolkit.
Now it works so much better.

@kalwalt
Copy link
Member

kalwalt commented May 19, 2021

@jalamprea You should check the brave robot example but yes probably the basic gltf example need also these changes,
I want to point out again that is not an issue with the tracking algorithm, but only with the rendering engine, in this case Three.js; because we plan to develop a library that is not rely to a specific rendering engine we are removing the ThreejsRenderer class and developing an external package for this see PR #180

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed 🚀 enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants