OpenXcom Forum

Contributions => Programming => Topic started by: Yankes on November 25, 2013, 02:17:42 am

Title: Replacing Text::paletteShift
Post by: Yankes on November 25, 2013, 02:17:42 am
Nice job, surprised to see it actually works. :)

I noticed your text colors seem off though. You probably need to update Surface::paletteShift to copy SDL_Color's new alpha field. There might be other places where you need to check this (I assume you already corrected the palettes to be fully opaque).
how about dropping `Surface::paletteShift` all together? I have test branch that replace it with custom blit function.
I dont publish it because of feature freeze before 1.0 and low performance grain (new function is faster that old one but because paletteShift isnt used offten, its hard to see difference).

Mod Edit: Split discussion since it's not Android-related.
Title: Re: Replacing Text::paletteShift
Post by: SupSuper on December 05, 2013, 06:59:51 pm
how about dropping `Surface::paletteShift` all together? I have test branch that replace it with custom blit function.
I dont publish it because of feature freeze before 1.0 and low performance grain (new function is faster that old one but because paletteShift isnt used offten, its hard to see difference).
As long as it doesn't break anything. :P
Title: Re: Replacing Text::paletteShift
Post by: Yankes on December 05, 2013, 10:28:26 pm
As long as it doesn't break anything. :P
you can test it yourself that didn't do it :)
https://github.com/Yankes/OpenXcom/tree/NewTextPrint
Title: Re: Replacing Text::paletteShift
Post by: SupSuper on December 20, 2013, 03:46:08 am
you can test it yourself that didn't do it :)
https://github.com/Yankes/OpenXcom/tree/NewTextPrint
It breaks the Battlescape text.
Title: Re: Replacing Text::paletteShift
Post by: Yankes on December 21, 2013, 03:02:42 pm
Yes, recently I spot this too :) Is already fixed.
Btw in orginal when you select aim type you can see some dark pixels separate form rest of characters. I think I could blend or remove them.

Some screenshots form hack that remove darkest pixels from fonts (this is far form proper fix but it can work as example)
Title: Re: Replacing Text::paletteShift
Post by: SupSuper on December 21, 2013, 06:03:00 pm
I think the darkest pixels help keep the text from blending into the background. Anyways post-editing the fonts should probably be avoided as they are user-editable and people can just change the PNG themselves if they want an alternate look (like the Amiga fonts).

Also I don't see a fix in your branch.
Title: Re: Replacing Text::paletteShift
Post by: Yankes on December 22, 2013, 01:08:58 am
Ok, branch get updated.
Title: Re: Replacing Text::paletteShift
Post by: SupSuper on December 22, 2013, 06:46:09 am
Looks good, merged.

Edit: Reverted, it breaks "soldier hair colors" mod (they become invisible).
Title: Re: Replacing Text::paletteShift
Post by: Yankes on December 22, 2013, 03:37:55 pm
Looks good, merged.

Edit: Reverted, it breaks "soldier hair colors" mod (they become invisible).
I see where I failed :/ Hint for future, dont do two things at once. I was in the middle of improving "soldier hair colors" that cause that bug to disappeared (I didnt even saw it).
After cherry picking commits I bring it back. Sorry for that SupSuper, I will create proper commit.
Title: Re: Replacing Text::paletteShift
Post by: Yankes on December 22, 2013, 04:08:52 pm
https://github.com/Yankes/OpenXcom/tree/NewTextPrint <- new fixed commit (old ones was trashed)
Title: Re: Replacing Text::paletteShift
Post by: SupSuper on December 22, 2013, 07:46:08 pm
Thanks, it's in.

Btw someone pointed me to an issue in your FixedFloat.h:

Engine/FixedFloat.h(59): warning C4244: '=' : conversion from 'const double' to 'long', possible loss of data

Shouldn't you be using a int64_t instead of a long?
Title: Re: Replacing Text::paletteShift
Post by: Yankes on December 22, 2013, 09:04:46 pm
I will look into it.
Title: Re: Replacing Text::paletteShift
Post by: Yankes on December 23, 2013, 02:21:43 am
Intended sized was int32, line 59 lack only cast to long.
And now fun part, when I wrote it back then I didnt know that shift `-1 << OFF` cause undefined behavior. But after some lurking on https://www.open-std.org/pipermail/ub/ (mailing list of standard comity of C++ working on undefined behavior) show that I was wrong.

After some thoughts I think best way to fix it is remove this file all together :)
It was used in experimental globe shading but finally I didnt used it. I include it because possible future use, but after more than year nobody used it (except for `pch.h` that include all headers).
Same history is for `ShaderRotate`.

This is branch that remove this headers (form VS files and makefiles too):
https://github.com/Yankes/OpenXcom/tree/RemoveTrash
Title: Re: Replacing Text::paletteShift
Post by: SupSuper on December 26, 2013, 07:57:17 am
Merged.