by Player701 » Wed Dec 18, 2019 9:21 am
This issue manifests only in the alternative HUD. Some frames of these icons look distorted because not all of them are of the same size. The method
AltHud::DrawImageToBox forces them into a fixed-size box; to perform this operation, the method also needs to know the texture's width and height. To retrieve them, it calls TexMan::GetScaledSize, which always returns the size of the first frame. As a result, if the size of the current frame is not the same as the size of the first frame, that frame will not be drawn as expected.
Ultimately, this means that in any drawing operation, calculations that depend on the properties of a texture might not always be valid when the texture is animated and at least one frame has a different size than the rest. Some way to access the properties of the current animation frame should be made available.
IMO, there are at least two ways to resolve this:
- Add a method to TexMan to allow retrieving the current frame of an animated texture (the animation is defined by the name of its first frame). Rewrite AltHud::DrawImageToBox to use this method. IMO, this is not a very clean solution, since it basically fully exposes the animation abstraction to user code, forcing the user to deal with it entirely manually. It also undermines the purpose of the DI_DONTANIMATE flag for BaseStatusBar::DrawImage/DrawTexture and the "animate" argument of Screen::DrawTexture.
- Add an extra bool argument to all TexMan methods that retrieve the properties of a texture to take animation into account. If the argument is true, the requested parameter(s) (size, offsets etc.) are returned for the current animation frame instead of the requested texture. This will require more work to be done on the engine side, but in the end dealing with animated textures will be more transparent for the user.
I'm not very familiar with the texture subsystem in the engine, so I have no idea how hard it might be to implement any of the above. Perhaps there is an even better solution which I've overlooked.
UPD: Further analysis based on the research of the corresponding parts of the source code:
The implemenations of most of TexMan's methods rely on
FTextureManager::ByIndex, which already has an animate parameter built into it. Looking at
how it's implemented, both solutions 1 and 2 are feasible implementation-wise. However, solution 1 would require altering existing code that makes use of TexMan. Solution 2 could avoid that by introducing an "animate" argument to the following methods, all of which make use of ByIndex internally:
- GetName
- GetSize
- GetScaledSize
- GetScaledOffset
- CheckRealHeight
If the new argument is set to true by default, then existing code need not be fixed. However, it might lead to breakage of user code that draws textures with animation disabled. This can be counteracted by versioning the change (if possible).
This issue manifests only in the alternative HUD. Some frames of these icons look distorted because not all of them are of the same size. The method [url=https://github.com/coelckers/gzdoom/blob/e21c9e0ef890e101b5b1951ec607895a3fdd7704/wadsrc/static/zscript/ui/statusbar/alt_hud.zs#L84-L111]AltHud::DrawImageToBox[/url] forces them into a fixed-size box; to perform this operation, the method also needs to know the texture's width and height. To retrieve them, it calls TexMan::GetScaledSize, which always returns the size of the first frame. As a result, if the size of the current frame is not the same as the size of the first frame, that frame will not be drawn as expected.
Ultimately, this means that in any drawing operation, calculations that depend on the properties of a texture might not always be valid when the texture is animated and at least one frame has a different size than the rest. Some way to access the properties of the current animation frame should be made available.
IMO, there are at least two ways to resolve this:
[list=1][*]Add a method to TexMan to allow retrieving the current frame of an animated texture (the animation is defined by the name of its first frame). Rewrite AltHud::DrawImageToBox to use this method. IMO, this is not a very clean solution, since it basically fully exposes the animation abstraction to user code, forcing the user to deal with it entirely manually. It also undermines the purpose of the DI_DONTANIMATE flag for BaseStatusBar::DrawImage/DrawTexture and the "animate" argument of Screen::DrawTexture.
[*]Add an extra bool argument to all TexMan methods that retrieve the properties of a texture to take animation into account. If the argument is true, the requested parameter(s) (size, offsets etc.) are returned for the current animation frame instead of the requested texture. This will require more work to be done on the engine side, but in the end dealing with animated textures will be more transparent for the user.[/list]
I'm not very familiar with the texture subsystem in the engine, so I have no idea how hard it might be to implement any of the above. Perhaps there is an even better solution which I've overlooked.
[b]UPD:[/b] Further analysis based on the research of the corresponding parts of the source code:
The implemenations of most of TexMan's methods rely on [url=https://github.com/coelckers/gzdoom/blob/e6f6f10e81735d4c1038e4241521d3b9b50007d5/src/gamedata/textures/textures.h#L564-L567]FTextureManager::ByIndex[/url], which already has an animate parameter built into it. Looking at [url=https://github.com/coelckers/gzdoom/blob/e6f6f10e81735d4c1038e4241521d3b9b50007d5/src/gamedata/textures/textures.h#L540]how it's implemented[/url], both solutions 1 and 2 are feasible implementation-wise. However, solution 1 would require altering existing code that makes use of TexMan. Solution 2 could avoid that by introducing an "animate" argument to the following methods, all of which make use of ByIndex internally:
[list][*]GetName
[*]GetSize
[*]GetScaledSize
[*]GetScaledOffset
[*]CheckRealHeight[/list]
If the new argument is set to true by default, then existing code need not be fixed. However, it might lead to breakage of user code that draws textures with animation disabled. This can be counteracted by versioning the change (if possible).