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

Fix non-even decal rendering #624

Merged
merged 3 commits into from
Oct 9, 2023

Conversation

horizon-wings
Copy link
Contributor

Decal rendering is currently broken on decals whose dimensions are non-even (not a multiple of two). This PR patches rendering calls from decal image components so that they behave as expected.

Decals are drawn with MTexture.DrawCentered(), which centers the decal on its Position. If the center of the decal is non-integer, the pixels of the image are not aligned to integer values, which results in decals being rendered in a way that no longer resembles the source image. My approach is to forcibly offset the rendering position of the image so that the image is "realigned". Any non-rotated decal whose scaled dimensions are even integers should be entirely unaffected; however, decals rotated at non-90 degree angles or with exotic scaling will render differently than before.

I am a little worried about performance since there can be a lot of decals onscreen in heavily-decoed areas, and this patch is adding quite a bit to each decal's Render. Anecdotally, it didn't seem very noticeable in a quick walk through an SJ lobby, but I can see it adding up.

I did not exhaustively test every possible combination of decal attributes, so help with that would be appreciated. I eyeballed a few basic combinations of scale and rotation, as well as checking the vanilla versions of mirror, banner, and core swap decals, and nothing is obviously broken.

This is my first time writing such a large amount of IL and also using MonoMod patching, so correction is appreciated if I did something silly. (In hindsight I probably should have just written a method and called it.)

@DemoJameson
Copy link
Member

@microlith57 can you take a look, since you mentioned the issue in #516

@microlith57
Copy link
Member

will do

@Kalobi
Copy link
Member

Kalobi commented Aug 9, 2023

This should probably also be applied to FlagSwapImage (added by Everest), which also draws decals using DrawCentered. CoreSwapImage (vanilla) also uses it, but is never used for custom decals (the coreSwap attribute just creates a FlagSwapImage), so there's no need to add extra code to rendering vanilla core decals.

@horizon-wings horizon-wings force-pushed the fix-non-even-decal-rendering branch from fa7ecd0 to 829a154 Compare September 3, 2023 17:50
@horizon-wings
Copy link
Contributor Author

horizon-wings commented Sep 3, 2023

@Kalobi it actually does apply the fix to rendering position for FlagSwapImage, just without the MonoMod attribute - since it's an Everest class, the change is directly in FlagSwapImage.Render(). Though if custom decals never use CoreSwapImage, does that mean the render patch attribute should be removed from there?

Edit: Whoops, I should test things before pushing them.

@Kalobi
Copy link
Member

Kalobi commented Sep 3, 2023

Oh, I somehow missed the change to FlagSwapImage. And yeah, it's probably not the worst idea to remove the patch from CoreSwapImage. The fewer useless patches, the better.

@horizon-wings
Copy link
Contributor Author

Made the CoreSwapImage change since I finally had time to "test" that. @microlith57 if you're able to take a look that would be lovely.

@microlith57
Copy link
Member

had a look; code looks reasonable, and worked on various non-even decals i tried it on. however it seems recolouring isn't in this PR's history (i think because of the force push), and I think merging it would clobber that.
i'd be ok with merging this after that's fixed 👍

@horizon-wings
Copy link
Contributor Author

As far as I can tell, decal recoloring is in the PR branch's history? I'm not 100% sure I'm looking in the right place and this is somewhat beyond my git skill level, though.

@microlith57
Copy link
Member

huh weird, apparently the way i checked out the branch messed things up somehow. sorry for the false alarm i guess
checking it out properly resulted in correct decal recolouring

@Kalobi Kalobi merged commit 275405c into EverestAPI:dev Oct 9, 2023
maddie480 added a commit that referenced this pull request Oct 19, 2023
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.

4 participants