search

yellowstonegames / SquidLib

Useful tools for roguelike, role-playing, strategy, and other grid-based games in Java. Feedback is welcome!
Other
454 stars 46 forks source link

SquidPanel.tint() doesn't line up with glyphs #198

Closed SquidPony closed 6 years ago

SquidPony commented 6 years ago

When running a .tint() effect it seems to be a few pixels lower than the original glyph

This might be due to the text cell factory doing resize adjustments that tint doesn't take into account, not sure

tommyettinger commented 6 years ago

Aaaaaagh... This bug has happened before, in SquidLayers. Is this in SquidLayers or SparseLayers? I need to be very careful about how this works because it can be rather difficult to fix, and it affects Dungeon Mercenary if I break something. I'll start investigating...

tommyettinger commented 6 years ago

A potential fix is to use SparseLayers if you only tint the background. I'll try to add foreground tint to SparseLayers next; SparseLayers should be much better about consistent positioning than SquidPanel or SquidLayers.

smelc commented 6 years ago

Indeed a bug of this kind affected me in SquidPanel a while ago.

tommyettinger commented 6 years ago

@smelc , I don't think there's a regression here, though it's possible... rather, the fix as applied may only work for some fonts. There is a good solution for this, I think, and it should be compatible with most if not all of the existing API, but I'm not sure if it will work. I think I can switch out one-char Label Actors for TextCellFactory.Glyph Actors, which use the same layout as normal grid-based chars instead of Label's potentially different rules (which can change with libGDX version changes, and have before). I might also be able to make a subclass of Label for showing multi-char Strings as Actors using TextCellFactory, which would probably be able to be dropped-in-place to replace Label. Let me know if any of this could cause breakage; I know Dungeon Mercenary has some intensive usage of different text layout.

tommyettinger commented 6 years ago

This appears fixed now, though I need to verify that the fix doesn't break any other effects. Now instead of the Actor-based methods in SquidPanel returning either an Image or a Label, they return a TextCellFactory.Glyph, which draws like normal text does in TextCellFactory (the same as before and after the tint) and can produce a solid block or a single char. Glyph is still an Actor, and some of its details needed to be reworked in the latest commit so all the color-changing Action types can affect it like any other Actor (mostly, the float field called color that was in Glyph has been replaced by the Color field called color in Actor). No lines of SparseDemo or BasicDemo2 needed to be changed by the internal changes to Glyph, so I'm reasonably confident this should be an easy upgrade, if it needs code changes at all. If there are performance changes, I haven't noticed any on desktop; even with all physical cores occupied with an unrelated RNG test, I get 700-900 FPS with integrated graphics on SparseDemo and over 400 FPS on BasicDemo2, and this is similar to before the change, if not better once the extra CPU usage is accounted for. I'll try updating Epigon to use this latest change and see how it goes...