fabiangreffrath / crispy-doom

Crispy Doom is a limit-removing enhanced-resolution Doom source port based on Chocolate Doom.
https://fabiangreffrath.github.io/crispy-homepage
GNU General Public License v2.0
808 stars 132 forks source link

Strife: add true color support #1183

Closed JNechaevsky closed 7 months ago

JNechaevsky commented 8 months ago

This is the final chapter of Crispy true color story, what else can be said? ๐Ÿ™‚ Just a few remarks and TODOs:

And of course, small and not very colorful example of before and after:

![image](https://github.com/fabiangreffrath/crispy-doom/assets/21193394/d4abcac0-5ba1-44a3-9bb0-db7bad36c624) ![image](https://github.com/fabiangreffrath/crispy-doom/assets/21193394/056bbaff-4f8b-42db-9a34-b3b5cafcfa70)
fabiangreffrath commented 8 months ago
  • TODO: Screen wiping disabled. It is not possible to use XLATAB neither for crossfading, nor for any kind of translucency. Wiping effect will require entirely different approach, probably via some SDL functions, like making a copy of existing texture and blending it with variable opacity level, depending on game tic or something. No idea how to proceed at the moment.

No need for low level functions. We know RGB pixel values for source and destination pixels and can blend them with each other.

  • TODOs : Graphical startup with RAW patches working, but not sure it's an optimal way, since pal_color[] must be filled before graphical startup, and then filled again after generating colormaps.

No problem. ๐Ÿคท

JNechaevsky commented 8 months ago

No need for low level functions. We know RGB pixel values for source and destination pixels and can blend them with each other.

May I ask for small advice regarding usable function? At the moment I know only how to operate with / blend single colors, while this case with whole screen/texture is far more complicated.

No problem. ๐Ÿคท

I'm afraid we do have a problem here. Intro sequence was slightly broken in Crispy Strife 6.0, i.e. it's simply not appearing, probably because of RAW patches are supposed to use byte, not pixel_t type just like RAW screens. The problem is - turns out, I have fixed only half of this problem, so only last frame of last sequence now appears. Hmm.

fabiangreffrath commented 8 months ago

May I ask for small advice regarding usable function? At the moment I know only how to operate with / blend single colors, while this case with whole screen/texture is far more complicated.

Sorry if I'm under-estimating things, but don't we just need something like I_BlendOver() with a variable blend_alpha?

JNechaevsky commented 8 months ago

Makes a huge sense, thank you. On paper, crossfading should be something like this when wiping routine is called. At least, as I can see this logics...

void F_CrossFade (void)
{
    // Make a copy of existing texture, once.
    pixel_t *dest = I_VideoBuffer[SCREENWIDTH * SCREENHEIGHT]; 

    // Stop crossfading when value equals zero.
    int opacity = 255;

    do 
    {
        if (--opacity == 0)
            continue;  // Stop when reached zero value.

        I_BlendFade(dest, I_VideoBuffer[SCREENWIDTH * SCREENHEIGHT], opacity);
    } while (oldgametic < gametic); // Must be framerate independent.
}
JNechaevsky commented 7 months ago

This seems to be working as cross-fading effect, we even don't need a new blending function:

```diff +++ crispy-doom/src/strife/f_wipe.c Fri Apr 5 08:14:40 2024 @@ -19,6 +19,7 @@ #include #include "z_zone.h" +#include "v_trans.h" // [crispy] blending functions #include "i_video.h" #include "v_video.h" #include "m_random.h" @@ -37,22 +38,22 @@ // when zero, stop the wipe static boolean go = 0; -static byte* wipe_scr_start; -static byte* wipe_scr_end; -static byte* wipe_scr; +static pixel_t* wipe_scr_start; +static pixel_t* wipe_scr_end; +static pixel_t* wipe_scr; void wipe_shittyColMajorXform -( short* array, +( dpixel_t* array, int width, int height ) { int x; int y; - short* dest; + dpixel_t* dest; - dest = (short*) Z_Malloc(width*height*2, PU_STATIC, 0); + dest = (dpixel_t*) Z_Malloc(width*height*sizeof(*dest), PU_STATIC, 0); for(y=0;y not ready to scroll yet) @@ -163,8 +168,8 @@ int dy; int idx; - short* s; - short* d; + dpixel_t* s; + dpixel_t* d; boolean done = true; width/=2; @@ -181,8 +186,8 @@ { dy = (y[i] < 16) ? y[i]+1 : 8; if (y[i]+dy >= height) dy = height - y[i]; - s = &((short *)wipe_scr_end)[i*height+y[i]]; - d = &((short *)wipe_scr)[y[i]*width+i]; + s = &((dpixel_t *)wipe_scr_end)[i*height+y[i]]; + d = &((dpixel_t *)wipe_scr)[y[i]*width+i]; idx = 0; for (j=dy;j;j--) { @@ -190,8 +195,8 @@ idx += width; } y[i] += dy; - s = &((short *)wipe_scr_start)[i*height]; - d = &((short *)wipe_scr)[y[i]*width+i]; + s = &((dpixel_t *)wipe_scr_start)[i*height]; + d = &((dpixel_t *)wipe_scr)[y[i]*width+i]; idx = 0; for (j=height-y[i];j;j--) { @@ -227,7 +232,7 @@ int width, int height ) { - wipe_scr_start = Z_Malloc(SCREENWIDTH * SCREENHEIGHT, PU_STATIC, NULL); + wipe_scr_start = Z_Malloc(SCREENWIDTH * SCREENHEIGHT * sizeof(*wipe_scr_start), PU_STATIC, NULL); I_ReadScreen(wipe_scr_start); return 0; } @@ -240,7 +245,7 @@ int width, int height ) { - wipe_scr_end = Z_Malloc(SCREENWIDTH * SCREENHEIGHT, PU_STATIC, NULL); + wipe_scr_end = Z_Malloc(SCREENWIDTH * SCREENHEIGHT * sizeof(*wipe_scr_end), PU_STATIC, NULL); I_ReadScreen(wipe_scr_end); V_DrawBlock(x, y, width, height, wipe_scr_start); // restore start scr. return 0; @@ -268,7 +273,7 @@ { go = 1; // haleyjd 20110629 [STRIFE]: We *must* use a temp buffer here. - wipe_scr = (byte *) Z_Malloc(width*height, PU_STATIC, 0); // DEBUG + wipe_scr = (pixel_t *) Z_Malloc(width*height, PU_STATIC, 0); // DEBUG //wipe_scr = I_VideoBuffer; (*wipes[wipeno*3])(width, height, ticks); } ```

Well, by "seems" I meaning that initial fading right after game start is working, but once it's done, the game soft-locks. This is exactly the same problem when XLATAB is composed without necessary precision in paletted render, I've tried it couple of years ago. Something must be done with these lines, probably.

Anyways, crossfading effect in super slow motion, it's not that epic, but at least colors seems to be fine:

https://github.com/fabiangreffrath/crispy-doom/assets/21193394/408cffd2-0186-4cc9-b5ea-44f68653dffe

JNechaevsky commented 7 months ago

Almost done with crossfading effect. So, the problem with crossfade is described on DoomWiki, and it also affects TrueColor render. I don't see any ways to fix it in initial implementation, but after some experiments found that fix is pretty simple - an infinite loop happens here, when render "fails" to compare one screen data with another: https://github.com/fabiangreffrath/crispy-doom/blob/master/src/strife/f_wipe.c#L99.

A small trick can help escaping such loop, here's a small example just for show:

```diff +++ R:/GITHUB/crispy-doom/src/strife/f_wipe.c Mon Apr 8 09:09:16 2024 @@ -42,7 +42,6 @@ static pixel_t* wipe_scr_end; static pixel_t* wipe_scr; + // [crispy] Amount of tics for safe performing a cross-fade effect. Full length takes 13 tics. + // This will fix vanilla bug with non-precise XLATAB and make possible TrueColor crossfading. +int fade_safe_tics; void wipe_shittyColMajorXform @@ -95,11 +94,10 @@ int pix = width*height; int i; boolean changed = false; + // wipe_shittyColMajorXform called every game (not frame) tic, so we can reduce it here, + // while it's armed to 13 in d_main.c, right before wiping process is started. + fade_safe_tics--; for(i = pix; i > 0; i--) { - if(*cur_screen != *end_screen) + if(*cur_screen != *end_screen && fade_safe_tics) // <- Here safe tics working as fail-safe counter. { changed = true; #ifndef CRISPY_TRUECOLOR *cur_screen = xlatab[(*cur_screen << 8) + *end_screen]; #else *cur_screen = I_BlendOverXlatab(*cur_screen, *end_screen); #endif } ```

Ah yes, also this thing... I'm afraid I don't get it right, still not very good with memory handling, but this way it works.

```diff // initial stuff if (!go) { go = 1; // haleyjd 20110629 [STRIFE]: We *must* use a temp buffer here. +#ifndef CRISPY_TRUECOLOR wipe_scr = (pixel_t *) Z_Malloc(width*height, PU_STATIC, 0); // DEBUG +#else + // [crispy] Perform everything in common buffer, otherwise malloc errors will happen. + wipe_scr = I_VideoBuffer; +#endif (*wipes[wipeno*3])(width, height, ticks); } ```

I'll pull changes soon, but what makes me really sad - despite of working more or less okay, this crossfade looks a bit... Poor. It could be more nice and soft, but at least there is some result.

JNechaevsky commented 7 months ago

@fabiangreffrath, just FYI, a progress report. Crossfading effect is done and working, I was aiming for something decent, but got identical to vanilla instead, except with cleaner TrueColor colors. Not sure if it is possible to make animation smoother since animation is performed via game tics, which makes a strict limitation in duration itself. Any ways, even possible infinite loop problem is fixed for both paletted and TrueColor renders - it's not really needed in first one, until custom XLATAB is used, but absolutely needed for TrueColor, since we comparing screen data between "old" and "new" screens.

Only one small but stinking problem remains: a startup sequence (animation of robot shooting to civilian). At the moment, it's showing just a black screen in paletted render (presumably, it was was broken even before TC code), and after some pushing, it's showing just a final frame in TrueColor render. Not sure how to properly proceed with both cases yet, TC obliviously requires pal_color array to be set, and it is done right before animation, but still doesn't work for non-final frames. And there have to be two calls for R_InitPalColors(), otherwise ingame pal_color graphics will be all black. ๐Ÿ˜

I don't want to leave it "as is" and try to push harder, but the game itself is now working just fine, though some extra testing is still needed.

JNechaevsky commented 7 months ago

Guess it's all done and ready for full review. Small testing map for checking translucency + translation effects: strife-test.zip (also contains Sigil piece for checking "invul" colormap effect).

Small remark: smooth crossfading is possible, and it looks much, much better. But I've bumped into problem of "the smoother fading is, the longer after-fading delay is happening". Something have to be done in wiping do loop probably, but not sure what exactly... Video demonstration, note that I'm holding "turn right" which is happening only after this delay is gone:

https://github.com/fabiangreffrath/crispy-doom/assets/21193394/df20a61a-04a7-4c94-8a57-e567e17152b1

JNechaevsky commented 7 months ago

My greatest pleasure, thank you for accepting and help!

Would like to take a small speech in the end. Heretic TrueColor took 10 days, Hexen took about 7. Strife in general part took ~24 hours if not counting reviewing, polishing and experimenting. Unforeseen "break" knocked me out for many of days, but this is where phrase "we are not in rush" gorgeously comes in and fits perfectly. โ˜๏ธ๐Ÿ™‚

Startup sequence is also fixed for both renders, but let's not point a finger who slightly broke it while adding negative gamma-correction levels. ๐Ÿ˜ถ

Kind thanks to @ceski-1 for massive efforts on Crispy Strife and especially for taking care of framerate-independent wipe effect and adding support for smooth diminished lighting - this is how TrueColor appears in all it's power! ๐Ÿค

The only thing that makes me sad at this point is a not smooth, but at least vanilla-like and soft-lock safe crossfade effect. Once smooth effect was seen, it was nothing more like "whoah!... ๐Ÿ˜ฎ", even if CPU is not very happy with such amount of computing. I will keep investigating, there have to be way.

Guess, first paragraph of FAQ needs small update now.

fabiangreffrath commented 7 months ago

Guess, first paragraph of FAQ needs small update now.

Please feel free! There will be another surprise: https://github.com/fabiangreffrath/crispy-doom/pull/1196

JNechaevsky commented 7 months ago

Please feel free! There will be another surprise: https://github.com/fabiangreffrath/crispy-doom/pull/1196

๐Ÿคฉ Yeah! Will gladly do, in the evening or at least on weekend, but anyways, better will show it to you first, for small proof-reading.

Guess, it also deserves a small announcement on DoomWorld forum thread, along with small explanations that "true color is not about new graphical effects or dynamic lighting. It's about clean colors in vanilla render". Will you take a honor to make this speech?

fabiangreffrath commented 7 months ago

๐Ÿคฉ Yeah! Will gladly do, in the evening or at least on weekend, but anyways, better will show it to you first, for small proof-reading.

Never mind, I think we will only have to remove the first sentence. ๐Ÿ˜‰

Guess, it also deserves a small announcement on DoomWorld forum thread, along with small explanations that "true color is not about new graphical effects or dynamic lighting. It's about clean colors in vanilla render". Will you take a honor to make this speech?

I'd like to announce this as part of the next "official" release. People closely following the project will notice earliert anyway.

JNechaevsky commented 7 months ago

Never mind, I think we will only have to remove the first sentence. ๐Ÿ˜‰

Done! โšก๏ธ

I'd like to announce this as part of the next "official" release. People closely following the project will notice earliert anyway.

Ah, sure, whatever you say.

JNechaevsky commented 7 months ago

Small and brain-dead trick for smoother crossfading without post-effect delay:

```diff +++ crispy-doom/src/strife/d_main.c Sat Apr 20 13:08:36 2024 @@ -410,15 +410,15 @@ do { do { nowtime = I_GetTime (); tics = nowtime - wipestart; I_Sleep(1); - } while (tics < 3); // haleyjd 08/23/2010: [STRIFE] Changed from == 0 to < 3 + } while (tics < 1); // proceed faster for more smooth effect // haleyjd 08/26/10: [STRIFE] Changed to use ColorXForm wipe. wipestart = nowtime; done = wipe_ScreenWipe(wipe_ColorXForm , 0, 0, SCREENWIDTH, SCREENHEIGHT, tics); I_UpdateNoBlit (); M_Drawer (); // menu is drawn even on top of wipes ``` ```diff +++ crispy-doom/src/strife/f_wipe.c Sat Apr 20 13:24:24 2024 @@ -73,15 +73,15 @@ ( int width, int height, int ticks ) { memcpy(wipe_scr, wipe_scr_start, width*height*sizeof(*wipe_scr)); // [crispy] arm fail-safe crossfade counter with // 13 screen transitions, "zero" count won't be used. - fade_counter = 14; + fade_counter = 255; // arm with full opacity return 0; } // // wipe_doColorXForm // // haleyjd 08/26/10: [STRIFE] @@ -99,25 +99,32 @@ int pix = width*height; int i; boolean changed = false; // [crispy] reduce fail-safe crossfade counter tics fade_counter--; + // please don't ask anything... + // this must mean: 13 screen translations multiplied by 3x wipe time + { + if (fade_counter < 255 - (13 * 3)) + fade_counter = 0; + } + for(i = pix; i > 0; i--) { if(*cur_screen != *end_screen && fade_counter) { changed = true; #ifndef CRISPY_TRUECOLOR *cur_screen = xlatab[(*cur_screen << 8) + *end_screen]; #else // [crispy] perform crossfading effect with 13 given opacity steps, multipled by 19: // 247, 228, 209, 190, 171, 152, 133, 114, 95, 76, 57, 38, 19 - *cur_screen = I_BlendOver(*end_screen, *cur_screen, fade_counter * 19); + *cur_screen = I_BlendOver(*end_screen, *cur_screen, fade_counter); #endif } ++cur_screen; ++end_screen; } ```

But aside from terrible magic number operation with fade_counter, it's still not good enough by making few last frames notably darker before crossfading ends. Turns out, I_BlendOver receiving slightly incorrect amount value (unlikely, simple printf showing that they are correct), or this is because effect in general is happening not with opaque background screen, but with translucent as well. Or something like that.๐Ÿคฏ

Another notable thing is - if using I_BlendAdd, controls are back imideatelly after "toxical" crossfading with fade_counter = 14 is done.