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

Add info about FastTESR and some other improvements #1

Merged
merged 2 commits into from Mar 22, 2017

Conversation

diesieben07
Copy link

No description provided.

Copy link
Owner

@elifoster elifoster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'm on mobile so i can't put a line comment.)

The very last sentence is a comma splice; the comma should be changed to a period:


A TESR can opt-in to being a FastTESR by extending the `FastTESR` class instead of `TileEntitySpecialRenderer`. Instead of implementing `renderTileEntityAt`, `renderTileEntityFast` must be implemented.

A FastTESR can offer performance improvements over a traditional TESR and should be used wherever possible. This is due to the fact that all FastTESR instances are batched together and only issue _one_ combined draw call for all FastTESRs per frame to the GPU. This advantage comes at the cost of making direct OpenGL access via `GlStateManager` or the `GLXX` classes impossible. Instead a FastTESR must only add vertices to the provided `VertexBuffer`, which represents the combined vertex data for all FastTESRs. This allows rendering `IBakedModel`s, an example can be found in Forge's `AnimationTESR`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you also need to override TileEntity.hasFastRenderer to true

@diesieben07
Copy link
Author

The very last sentence is a comma splice; the comma should be changed to a period
No, it is not. It's an example for rendering IBakedModels, which is not the only usecase for FastTESR.

@elifoster elifoster dismissed their stale review March 22, 2017 17:58

I'll just fix it, it's no problem.

@elifoster
Copy link
Owner

@diesieben07 I'm talking about grammar. This allows rendering IBakedModels, an example can be found in Forge's AnimationTESR. is two independent clauses.

@elifoster elifoster merged commit 571698f into elifoster:tesr Mar 22, 2017
@diesieben07
Copy link
Author

diesieben07 commented Mar 22, 2017

I know what you are talking about. I am not a native English speaker, but this comma seems correct to me. Because it is not two independent clauses.
Think of "This allows rendering IBakedModels, an example of which can be found in ...".

@elifoster
Copy link
Owner

elifoster commented Mar 22, 2017

If the example is specifically for rendering IBakedModels, it should be joined with a conjunction and a comma (in which case I'll fix my fix). However to me, with it as a splice or not, it sounds like it is simply providing an example of FastTESRs in general.

@diesieben07
Copy link
Author

It kind of is. Whatever, it doesn't matter. Just "This allows Rendering IBakedModels." as it's own sentence sounds stupid imho.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants