DrawImage enhancements

Post a reply

Smilies
:D :) :( :o :shock: :? 8-) :lol: :x :P :oops: :cry: :evil: :twisted: :roll: :wink: :geek: :ugeek: :!: :?: :idea: :arrow: :| :mrgreen: :3: :wub: >:( :blergh:
View more smilies

BBCode is OFF
Smilies are ON

Topic review
   

Expand view Topic review: DrawImage enhancements

Re: DrawImage enhancements

by Tesseract » Sat Sep 21, 2013 12:22 pm

Thank you very much for addition!
My apologies, it seems that offset was out of order indeed. I fixed the obvious ones and overlooked that. I was using recommended development tools (Visual C++,NASM) with default settings, I'm honestly suprised that such presumably important issue didn't spit any warning.
Well, that will be all, let's hope we'll soon see good uses for this additions!

By the way, I suppose backward compatibility dictates otherwise, but maybe that size limits could be useful in default Zdoom's fullscreen hud. Worth a question, at least.

Re: DrawImage enhancements

by Blzut3 » Sat Sep 21, 2013 11:24 am

Added with some minor changes. You didn't fix the constructor initializer list properly, but I guess your compiler doesn't warn you when they're out of order. I didn't check in detail but I believe the issue was your new members came after offset.

Re: DrawImage enhancements

by Tesseract » Thu Sep 19, 2013 1:09 pm

I've included all the changes you suggested.
Had no idea that order matters in initializer list, thanks for pointing it out! I've removed all TODO comments now, you can consider if moving GetInventoryIcon somewhere else in the file is good idea (as it's not weapon-specific function anymore).
As for DrawGraphic optimalization, I don't think it cause some precision side effects, so OK for me. Also, I just left there original formula, commented out, for code readablitity. I think it may be useful for another unlucky coder that will waste time triing to figure how exactly that line work. If you think it's trivial, feel free to remove it.
Attachments
drawimage.zip
(3.55 KiB) Downloaded 68 times

Re: DrawImage enhancements

by Blzut3 » Thu Sep 19, 2013 11:40 am

A few comments:

In C++ constructors, the initializer list needs to be specified in the same order as the variables are declared in the class. So it should look like:

Code: Select all

			translatable(false), flags(0), applyscale(false), type(NORMAL_IMAGE),
			image(-1), maxwidth(-1), maxheight(-1), spawnScaleX(1.0f),
			spawnScaleY(1.0f), offset(static_cast<Offset> (TOP|LEFT)),
			texture(NULL), alpha(FRACUNIT)
In CommandDrawImage::Draw you have:

Code: Select all

			else if (applyscale)
			{
				w=(int) (texture->GetScaledWidth()*spawnScaleX);
				h=(int) (texture->GetScaledHeight()*spawnScaleY);
			}
I believe these should be texture->GetScaled*Double()?

You have code that looks like:

Code: Select all

					FTextureID icon = GetInventoryIcon(ammo, flags, &applyscale);
					
					if (applyscale)
					{
						spawnScaleX = FIXED2FLOAT(ammo->scaleX);
						spawnScaleY = FIXED2FLOAT(ammo->scaleY);
					}
					
					texture = TexMan[icon];
Repeated 4 times, so it would probably be wise to throw that into a private function in the class.

The change in sbarinfo.cpp looks correct. It probably won't amount to any real performance difference, but you could factor out forceWidth or forceHeight as well to remove a floating point division:

Code: Select all

dx -= forceWidth*(0.5-(texture->GetScaledLeftOffsetDouble()/texture->GetScaledWidthDouble()));
I'll let you check if that gives undesired precision side effects or not.

Other then those, I think the patch looks fine. You can remove the TODOs above CommandDrawImage and CommandDrawSelectedInventory. They should be fine.

Re: DrawImage enhancements

by Tesseract » Wed Sep 18, 2013 5:20 pm

Good news, everyone!

After short time of coding and long time of bugfixing, this patch is done and ready!
Special thanks goes to Blzut for excellent discussion, both on and off forum.

Finalized syntax:

Code: Select all

DrawImage [translatable] <image>, <x>, <y> [, {lefttop|center|centerbottom}, [ width, height, [ flags]]];
DrawImage [translatable] <image>, <x>, <y> [, {lefttop|center|centerbottom}, [ width, height, [ flags|alternateonfail]]]
{
	//Block executed if nothing is drawn
}
[Else
{
	//Block executed if drawing succeeded
}]
"lefttop" is default alignment. It's the one used if you don't set this argument at all. Beware, as offsets are applied some sprites like pickups may end up drawn in unexpected places. (That also means I can't guarantee the position in the box with this alignment.)

Width and height fields set dimensions of box which limits maximal size of the drawn sprite, it's downscaled if too big.
Ideal for weapon/ammo/armoricon as they have variable sizes.

Flags are:

Code: Select all

{skipicon|skipalticon|skipspawn|skipready|alticonfirst|forcescale|alternateonfail}
  • *icon searching*

    weaponicon, armoricon and ammoicons goes through so-called icon-searching process.
    Best and most logical one is

    AltIcon (defined in ALTHUDCF) -> Actor Icon property -> sprite from Spawn state [for weapons: -> sprite from Ready state]

    That's how AltHud works, however, to maintain compatibility with current SBarInfo code, the default order is:

    Icon -> AltIcon -> Spawn [->Ready]

    Note that the searching process has been expanded for armor/ammo. No compatibility problems are expected there, but if they'd arise, it's easy fix.

    You can affect this process with these flags:
  • alticonfirst - AltHud (better) order of checking
  • skip* - skips one phase of checks

    *rest*
  • forcescale - sprite is upscaled to fit the box if too small
  • altternateonfail - similiar to syntax of DrawSelectedInventory
That's all. I've also included micromod for quick test - modified Legacy Hud (originally by DenisBelmondo) that can draw Legacy-esque ammo icons even for custom ammo types. Have fun!

NOTE: I've tested everything and I'm fairly sure everything works right, but I'm just a human (and don't forget average coder). So if there'll be any problems with this code, I'm very sorry and I'm going to fix the mess.

Edit: minor error in standalone fix for DrawGraphic spotted and fixed
Attachments
DrawImage.zip
(4.19 KiB) Downloaded 71 times
HUD-Legacy.pk3
(4.27 KiB) Downloaded 71 times

Re: DrawImage enhancements

by Tesseract » Tue Sep 10, 2013 7:45 pm

Blue Shadow wrote:
Tesseract wrote:probably only thing I can do about it is renaming it to more fitting "DrawImage enhancements", which could attract more people. I'm not sure how to do that myself, though.
Simply, use the Edit button on the opening post and change the title.
Thank you, didn't expect that!

Re: DrawImage skipReady enhancement

by Blue Shadow » Tue Sep 10, 2013 6:14 pm

Tesseract wrote:probably only thing I can do about it is renaming it to more fitting "DrawImage enhancements", which could attract more people. I'm not sure how to do that myself, though.
Simply, use the Edit button on the opening post and change the title.

Re: DrawImage skipReady enhancement

by Tesseract » Tue Sep 10, 2013 3:18 pm

I agree on all points. I didn't even know that ammo/armor don't use Spawn state sprites as fallbacks, this will be added along with ALTHUDCF defined icons, so icon searching process will be same for anything: Icon -> AltIcon -> Spawn -> [Ready]

As I said before I'm not so sure many hud creators would read this thread in the first place, probably only thing I can do about it is renaming it to more fitting "DrawImage enhancements", which could attract more people. I'm not sure how to do that myself, though.

Re: DrawImage skipReady enhancement

by Blzut3 » Tue Sep 10, 2013 11:00 am

Tesseract wrote:Well, unlike original submission, in proposed solution flags would affect any use of DrawImage that goes through icon searching process - armoricon,weaponicon,ammoicons at least. Still not entirely universal, but worth mentioning this wouldn't indeed be one-use feature. You probably knew that, but I think it's better to state it for the sake of completeness.
Oh good point. SBarInfo only go through the alternative icons process for weaponicon though. If you can make this generic for all icon types I'll add it. The tricky part is handling which flags are set by default.
Tesseract wrote:Actually, I tried to address this very issue by asking for public opinion. Something like poll with "Would you use these flags in your hud? Yes/No" question should give definitive answer if this is obscure choice or wanted standard. As you haven't mentioned this in your posts, I assumed you turned down this idea entirely, therefore addition wouldn't depend on suggestion popularity and would be declined only because it would be better in ZS. But if your ZS speculation is right, everything would be better in ZS and SBARINFO would be effectively deprecated. That led me to previous conclusion that Sbarinfo additions are not much welcomed.
I'm not sure exactly one would go about proving popularity of the feature. Probably the best way is to let this thread sit and if people bump it with "I'd like this too."

Re: DrawImage skipReady enhancement

by Tesseract » Tue Sep 10, 2013 6:46 am

Thank you for making your point clearer, this however introduces few things I'd like to discuss per statement.

Blzut3 wrote:As I stated my primary objection comes from the fact that these flags apply only to the case of DrawImage where weaponicon is used.
Well, unlike original submission, in proposed solution flags would affect any use of DrawImage that goes through icon searching process - armoricon,weaponicon,ammoicons at least. Still not entirely universal, but worth mentioning this wouldn't indeed be one-use feature. You probably knew that, but I think it's better to state it for the sake of completeness.
Blzut3 wrote:There's nothing technically wrong with that, but from that standpoint of "ZDoom doesn't actually support modifier mods," unless you can show that the flags would be widely used, I would rather not have the bloat there.
Actually, I tried to address this very issue by asking for public opinion. Something like poll with "Would you use these flags in your hud? Yes/No" question should give definitive answer if this is obscure choice or wanted standard. As you haven't mentioned this in your posts, I assumed you turned down this idea entirely, therefore addition wouldn't depend on suggestion popularity and would be declined only because it would be better in ZS. But if your ZS speculation is right, everything would be better in ZS and SBARINFO would be effectively deprecated. That led me to previous conclusion that Sbarinfo additions are not much welcomed.
Blzut3 wrote:I would rather not have the bloat there. Especially since I know trying to cater for modifiers will just lead down a road of requesting many similar small things that ZScript should, in theory, be able to solve in one go.
Can't argue with that. I can only repeat that these flags can be shortcut for (potentially) commonly used option, more elegant and quicker than eventual equivalent ZS code. It's up to developers to decide if it's enough to approve it.
Blzut3 wrote:I would have no objection to the maxwidth/maxheight or sub-block on failure suggestions mentioned for example.
Great, I'm glad we agree, at least on the less controversial part.

NOTE: Honestly, I've read through some recently WFDS'ed suggestions and it seems it's not equivalent of death sentence by now. You were right, ZS may be closer than one may think.

Re: DrawImage skipReady enhancement

by Blzut3 » Mon Sep 09, 2013 6:07 am

It's difficult for anyone other than Randy to say exactly what will in regards to the relationship between SBarInfo and ZScript.

I definitely wouldn't say that additions to SBarInfo are futile. As I stated my primary objection comes from the fact that these flags apply only to the case of DrawImage where weaponicon is used. There's nothing technically wrong with that, but from that standpoint of "ZDoom doesn't actually support modifier mods," unless you can show that the flags would be widely used, I would rather not have the bloat there. Especially since I know trying to cater for modifiers will just lead down a road of requesting many similar small things that ZScript should, in theory, be able to solve in one go.

I would have no objection to the maxwidth/maxheight or sub-block on failure suggestions mentioned for example.

Re: DrawImage skipReady enhancement

by Tesseract » Sun Sep 08, 2013 8:55 pm

Alright then, I suppose this got already decided.
DS/ZS as you outlined looks really nice and I'm looking for it, on the other hand I believe SBARINFO may still have it's use for less adept coders wanting simple huds.
Otherwise, your arguments and conclusion would imply that any additions to SBarInfo at this point are more or less futile. If that's the case, this information should be more well known.

Re: DrawImage skipReady enhancement

by Blzut3 » Sun Sep 08, 2013 8:03 pm

That is basically my conclusion.
Spoiler: ZScript speculation

Re: DrawImage skipReady enhancement

by Tesseract » Sun Sep 08, 2013 5:20 pm

Okay, so it seems that our final question is, WFDS or implement this as a flag?

DoomScript is, indeed, ultimate answer to all world's problems and most likely would allow to handle these options. As a coder I understand that needless addition of another feature is inefficient and makes maintainance difficult. Despite that, here are few points favouring flags:
  • I don't know DS specifications, not even sure anyone besides Randy do - so if you know more, please correct me on this one. I think that DS will be primarily C-like thing that substitutes/extends/unites Decorate and ACS. Full addition into special lumps wouldn't have much use, would be SBARINFO exception? It was designed to be simple, lightweight scripting lump that can't affect game state in any way, ensured by finite amount of own functions and lack of variables. These properties allows for example easy hud switching when loading games, and such advantages can't be seen in potential full, non SBARINFO based DoomScript huds, which are just evolution of ACS huds. So it's possible that it won't be changed much for sake of simplicity.

    Even if SBARINFO would get DS additions that only offer passive information retrieval, question is how difficult would be to replicate these flags there. Code would be certainly longer and with multiple conditionals. That's not a problem if such flags would be used in one or two huds, but if they would be in like 75% of them, flags are more optimal.

    Lastly, WFDS over the years started to look like "When it's done" from 3d Realms, leaving similiar aftertaste. As common Zdoom player, I see it as distant as it was decade ago. To say it pragmatically, I'd like to get my hud working in official releases before 2025. Indeed, it won't be added for one modder, so I think real question is how many hud creators would use these flags.
Therefore, my final opinion on this matter is this:
It was already demonstrated that features offered by these flags can be useful. If sufficient amount of modders would like to use them, to the point that their usage would be common practice or almost standard in some kinds of huds, it's better to make them flags, available right now and easy to type. On the contrary, if they'd be obscure choice used in bare minimum of cases, DoomScript is much better option.

Blzut, if you came to the same conclusion, I'll leave it to modders' consensus, maybe even start a poll or something. We can discuss how many hud creators would be sufficient to justify this addition. I'm just concerned if enough hud creators would actually read this thread.
Also, would your answer be enough or should I wait for Graf's opinion about this voting too?

In case this will be approved, I think I'm going to code it after all, shouldn't be too hard.
Spoiler: Slightly offtopic babble

Re: DrawImage skipReady enhancement

by Blzut3 » Thu Sep 05, 2013 8:25 pm

Thanks for the clarification. I'm still not entirely sold on the idea of the flags since ZDoom lacks a whole lot of "cross modding" features. I'm sure with little effort you could come up with a dozen examples of little paper cuts that prevent mods from working seamlessly with others (not limited to status bars). The question is do we bother even trying to go down this route or just WFDS where in theory one could actually have the proper information at their disposal?
Tesseract wrote:Blzut, thanks for your feedback, I'm only puzzled by your remarks about skipspawn being useful just for vanilla. In fact, I think the opposite, as all these flags are designed to increase cross-mod compatibility. Maybe it was just little misunderstanding and it's cleared now, if not, please explain it in more detail.
The point was such a hud would only be in full functionality on supported mods. In general this would mean just vanilla, but in your example you include Brutal Doom and Samsara.
Tesseract wrote:NOTE: I didn't even dare to suggest it,but even better than automatic "draw nothing on failure" would be if DrawImage could work as flow control element,in this fashion:
Actually it wouldn't be entirely unprecedented. DrawSelectedInventory has flag to execute a sub block if nothing is drawn. If you choose to implement something like this be careful though, drawswitchableimage hangs off of the drawimage code.

Top