fixed point macros can't be turned off anymore (GNA #4932) about wesnoth HOT 9 CLOSED

Near as I can tell, all this junk is being used in stuff having to do with pixel values R G B and Alpha. The idea is to do some of the math in a fixed-point representation where we have an int32_t holding a 24 bits left of the decimal and 8 bits to the right. Of course, it's sorta lame to be doing signed math on unsigned values, but whatever. Most of the uses are creating the values 0x00000000 and 0x00000100 (0.0 and 1.0).

In at least one place, the int32_t value is stored in an int16_t so I hope whoever did that made very sure to avoid any implicit sign extension errors.

Is this a code smell? Yes.

Is it screaming to be fixed? No. It doesn't seem to be causing any problems.

Is it premature optimization? Probably. But, considering where it's used, in the few places it actually does anything, it's probably actually speeding things up enough to be warranted.

My recommendation would be to hold our noses, leave things as they are and put the entire src/utils/math.hpp file on the list of good projects for a new programmer to work on because it's probably not causing issues, but it's just asking for cleanup and/or elimination.

from wesnoth.

Can this be closed?

• the OP is from 2005
• there is no src/utils.hpp any more.
• src/display.cpp has no display::draw_bar any more.

from wesnoth.

I seem to recall those macros being removed, as well.

from wesnoth.

These macros still exist:

And they are still being used by scale_surface().

from wesnoth.

Ah, okay. Well, presumably we can just remove the #if and stuff, then. I think changing it ti #if 1 satisfied the issue though.

from wesnoth.

Um. WTF? We seriously were doing a de-normalizing floating-point implementation? What, did one of the target machines lack a working floating-point unit?

I'd strongly suggest change #if 1 to #if 0, debug, then reduce until the macros meet their long-overdue demise.

from wesnoth.

I believe one of the target machines actually did lack a floating-point unit - OpenPandora possibly? Support for that has been dropped now, though.

from wesnoth.

My guess would be that the fixed-point implementation is used for performance.

from wesnoth.

Well, take the junk out and see if the game crawls.

While I agree, it's possible someone's attempt at an optimization, but if so it smells highly of premature optimization. I'd want a comment block with A/B test results proving a significant improvement. Otherwise, this is just an obscure way to hide basic errors.

from wesnoth.