Split more rendering aspects from ZoneRenderer #5742
Conversation
- Moved caches to class, added setters and static isLabelVisible method. - Moved token label generation in token rendering sequence preparatory to moving it out to DecorationRenderer. New class TokenDecorationRenderer - Paints things before and after token rendering. - Hub for halos, bars, states, etc. More bits to follow. New class OverlayRenderer - For painting states and bars. FacingArrowRenderer - Facing arrow now follows grid cell shape. Made some variables non-local. GeometryUtil - Added functions for finding intersection points. HaloRenderer - Now honours preference for opacity. TokenRenderer - Updated to call decoration renderer before and after painting image. plus other tweaks. ZoneViewModel - Added method- isUsingVision(). ZoneRenderer - Inverted hasMoveSelectionSetMoved(). - Made imageLabelFactory a constant. - LabelRenderingCache offloaded to LabelRenderer. - Renamed renderLabels() method as it is confusing. - Removed redundant image loading and token flipping from renderTokens(). - Other housework.
bubblobill
added
performance
Moved state opacity check to StateRenderer. Moved StateRenderer out of ZR and into TokenRenderer. Changed rendering for translucent tokens to be opaque on hover or selected.
…ecorationsOutOfZR
Opacity is now multiple of state opacity and token opacity
| private float lineWeight = AppPreferences.haloLineWidth.get(); | ||
|
|
||
| { | ||
| AppPreferences.haloOverlayOpacity.onChange(i -> opacity = i / 255f); |
There was a problem hiding this comment.
I do not think AppPreferences.haloOverlayOpacity is intended to be used in this way as the opacity for all rendered halos, but was intended just for a token's vision when using the halo color (as per the logic in VisionOverlayRenderer.java). Given the current default preference value of 50, this code would nerf everyone's halos!
Maybe the existing AppPreferences.haloOverlayOpacity display text should be changed for clarity (even though it sits under the Map Visuals tab).
In light of my halo feature PR, maybe there should instead be a new campaign setting or preference for global halo opacity (and/or configured for halos a la lights and auras)?
There was a problem hiding this comment.
Then they should not have called it "halo overlay opacity", and I agree the default is stoopid.
If only there was some way to manage halos in a fine-grained manner that permitted individual variation from the global setting. :P
There was a problem hiding this comment.
Just to echo what Baaaaaz said, don't change the behaviour of the preference in this way. Fix the wording if you don't like it.
This opacity setting should only apply to the vision overlay, not the halo around the cell. The two have nothing to do with each other, and this will only cause confusion.
| @Subscribe | ||
| private void onGridChanged(GridChanged event) { | ||
| if (event.zone() != null) { | ||
| this.zone = event.zone(); |
There was a problem hiding this comment.
You don't want to change the zone here since each FacingArrowRenderer is specific to a single zone (as each TokenRenderer / ZoneRenderer is). Instead, check that the zone is the same, and ignore the event if it's not.
This event handling is also unnecessary. isSquare isn't used and should be removed, and isIsometric is so cheap it's not worth managing state just to get it.
| return labelImageCache; | ||
| } | ||
|
|
||
| private final ZoneRenderer renderer; |
There was a problem hiding this comment.
Can remove renderer as it's no longer used.
| public class LabelRenderer implements ItemRenderer { | ||
|
|
||
| private static final Map<GUID, LabelRenderer> labelCache = new HashMap<>(); | ||
| private static final Map<GUID, BufferedImage> labelImageCache = new HashMap<>(); |
There was a problem hiding this comment.
The labelImageCache is a memory leak and needs to be changed. There needs to be some way to allow old entries to be garbage collected. The easy way to do this is to not make these static so that they can at least be collected when the zone is removed. Or you need a way to explicitly remove old entries. Compare with the original ZoneRenderer.labelRenderingCache which did both.
labelCache has similar issues. Although entries can be removed, it's only done as part of the render loop. So there's no way for entries from a removed zone to be collected.
| private float lineWeight = AppPreferences.haloLineWidth.get(); | ||
|
|
||
| { | ||
| AppPreferences.haloOverlayOpacity.onChange(i -> opacity = i / 255f); |
There was a problem hiding this comment.
Just to echo what Baaaaaz said, don't change the behaviour of the preference in this way. Fix the wording if you don't like it.
This opacity setting should only apply to the vision overlay, not the halo around the cell. The two have nothing to do with each other, and this will only cause confusion.
3 participants
Identify the Bug or Feature request
You can link a pull request to an issue with one of
fixes #5622 #5746
progresses #5594 #5586 #4726 #4944 #3691 #2750 #2346 #2328 #2000
until I got sick of looking for affected issues
Description of the Change
LabelRenderer
New class TokenDecorationRenderer
New class OverlayRenderer
FacingArrowRenderer
GeometryUtil
HaloRenderer
TokenRenderer
ZoneViewModel
ZoneRenderer
Possible Drawbacks
I'm not very good at my job
Documentation Notes
Separated most token decoration rendering out of ZoneRenderer
This change is