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 support for Iris's new text sink API #104

Merged
merged 4 commits into from
Jun 10, 2022

Conversation

SquidDev
Copy link
Member

@SquidDev SquidDev commented Jun 7, 2022

This is needed on Iris 1.2.5 as they change the vertex format of RenderType.text, so us chucking bytes in directly won't work!

I'd like to profile this before merging and check that the new virtual calls are JITted away, but otherwise this is good for review.

CCing @IMS212, in case they have any comments.

@SquidDev SquidDev requested a review from toad-dev June 7, 2022 21:43
@SquidDev SquidDev marked this pull request as draft June 7, 2022 21:43
Copy link
Member

@toad-dev toad-dev left a comment

Choose a reason for hiding this comment

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

Thank you so much Squid! I've been really busy and wasn't sure when I would be able to work on this. I only saw the one issue. Interested to hear about performance and JITWatch things. I learned quite a bit talking to you about optimization stuff last time.

When an index buffer grows beyond the 2^16 boundary, the type is
promoted from a SHORT to an INT. However, this type is not updated in
any of the existing DirectVertexBuffers, so we mis-interpret the index
buffer and render garbage.

To fix this, we now store the index buffer object instead (and also
acquire it during upload), and then read the type from that (which is
guaranteed to be always correct).

This is a little more aligned to how 1.19 does things (though 1.19 does
bind during upload rather than first draw).
@SquidDev
Copy link
Member Author

SquidDev commented Jun 8, 2022

Good enough (TM).

A screenshot of JITWatch showing quad being inlined

A flamechart showing perf is roughly the same, I guess??

A CC:R monitor showing Waver, using Iris shaders

@SquidDev SquidDev marked this pull request as ready for review June 8, 2022 19:38
@toad-dev
Copy link
Member

toad-dev commented Jun 8, 2022

Looks good to me 👍

@Merith-TK Merith-TK merged commit dc7a417 into mc-1.18.x/stable Jun 10, 2022
@SquidDev SquidDev deleted the feature/iris-api branch June 10, 2022 06:03
@SquidDev
Copy link
Member Author

Ahhh. I targetted the wrong branch for this. Don't know if you want me to recreate on top of /1.18.2, or just merge /stable into /1.18.2?

@Merith-TK
Copy link
Collaborator

gah! yeah just merge stable into 1.18.2

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