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

WebGLRenderer: Enable rendering without index and position attribute #13128

Merged
merged 2 commits into from
Jan 17, 2018

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Jan 17, 2018

Fixes #10751

We only use dataCount in order to determine drawEnd if an index or a position attribute is present. Otherwise, it will use Infinity inside the Math.min statement.

Like mentioned in the issue, it's necessary to specify drawRange (or group) if a geometry consists only of custom attributes.

@WestLangley
Copy link
Collaborator

If you are not going to use dataCount, there is no need to compute it in the previous lines.

Is this PR equivalent to just initializing dataCount to Infinity?

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jan 17, 2018

If we initialize dataCount to Infinity, then we might have problems with such setups: #8395 (comment)

If you are not going to use dataCount, there is no need to compute it in the previous lines.

How would you write it differently? If index is null and position is undefined, the value of dataCount ist still 0.

@WestLangley
Copy link
Collaborator

How would you write it differently?

By initializing dataCount to infinity and removing the remainder of your changes. I think it is the same.

I'm not sure if there is a downside to supporting this feature, however....

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jan 17, 2018

By initializing dataCount to infinity and removing the remainder of your changes. I think it is the same.

Yes, you're right. I've checked #8395 again. I was just concerned about empty Geometry objects. But when they are converted to BufferGeometry, they have at least a position attribute.

@mrdoob mrdoob merged commit c25584e into mrdoob:dev Jan 17, 2018
@mrdoob
Copy link
Owner

mrdoob commented Jan 17, 2018

Thanks!

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.

3 participants