-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Strife: add true color support #1183
Conversation
No need for low level functions. We know RGB pixel values for source and destination pixels and can blend them with each other.
No problem. 🤷 |
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.
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 |
Sorry if I'm under-estimating things, but don't we just need something like |
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...
|
This seems to be working as cross-fading effect, we even don't need a new blending function: +++ crispy-doom/src/strife/f_wipe.c Fri Apr 5 08:14:40 2024
@@ -19,6 +19,7 @@
#include <string.h>
#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<height;y++)
for(x=0;x<width;x++)
@@ -88,8 +89,8 @@
int height,
int ticks )
{
- byte *cur_screen = wipe_scr;
- byte *end_screen = wipe_scr_end;
+ pixel_t *cur_screen = wipe_scr;
+ pixel_t *end_screen = wipe_scr_end;
int pix = width*height;
int i;
boolean changed = false;
@@ -99,7 +100,11 @@
if(*cur_screen != *end_screen)
{
changed = true;
+#ifndef CRISPY_TRUECOLOR
*cur_screen = xlatab[(*cur_screen << 8) + *end_screen];
+#else
+ *cur_screen = I_BlendOverXlatab(*cur_screen, *end_screen);
+#endif
}
++cur_screen;
++end_screen;
@@ -134,8 +139,8 @@
// makes this wipe faster (in theory)
// to have stuff in column-major format
- wipe_shittyColMajorXform((short*)wipe_scr_start, width/2, height);
- wipe_shittyColMajorXform((short*)wipe_scr_end, width/2, height);
+ wipe_shittyColMajorXform((dpixel_t*)wipe_scr_start, width/2, height);
+ wipe_shittyColMajorXform((dpixel_t*)wipe_scr_end, width/2, height);
// setup initial column positions
// (y<0 => 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 Anyways, crossfading effect in super slow motion, it's not that epic, but at least colors seems to be fine: New.project.mp4 |
It's working technically now, but the problem with infinite loop of comparing old and new screen data have to be resolved.
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: +++ 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. // 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. |
Now it looks and feels like vanilla!
@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 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 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. |
1) No need for separated R_InitPalColors() function. (Re-)filling pal_color[] array must be done along with colormaps[] as it's related to gamma-correction. 2) Redraw side views of screen border on toggling gamma-correction.
src/i_video.c
Outdated
} | ||
|
||
// [crispy] Perform screen crossfading effect by given amount of opacity, used for Strife | ||
const pixel_t I_BlendOverCrossfade (const pixel_t bg, const pixel_t fg, const int amount) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think now we have reached the point where we can leave this as the only actual implementation of I_BlendOver()
and have all the other variants turned into macros with hard-coded amount
values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've read all of your remarks, but let's please start from this one. I've got your idea of simplification and reducing amount of existing duplications and opacity values. This should be something like this:
const pixel_t I_BlendOver (const pixel_t bg, const pixel_t fg, const int amount)
{
const uint32_t r = ((amount * (fg & rmask) + (0xff - amount) * (bg & rmask)) >> 8) & rmask;
const uint32_t g = ((amount * (fg & gmask) + (0xff - amount) * (bg & gmask)) >> 8) & gmask;
const uint32_t b = ((amount * (fg & bmask) + (0xff - amount) * (bg & bmask)) >> 8) & bmask;
return amask | r | g | b;
}
// This should replace Doom's I_BlendOver calls:
const pixel_t I_BlendOverTranmap (const pixel_t bg, const pixel_t fg)
{
// ... i.e. when I_BlendOverTranmap is called, catch it's values and pass it to main BlendOver function
return I_BlendOver(bg, fg, 0xa8); // 168
}
But... You mean exactly to macrocize such "passing" pixel_t
functions? Is it possible at all, without writing them in .c files? I never tried to write whole function as macro, so small hint or example will be very appreciated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like
#define I_BlendOverTranmap(a,b) I_BlendOver(a,b,0xa8)
with the corresponding comment in v_video.h
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't worked on my side. In Doom all I_BlendOver
calls replaced with I_BlendOverTranmap
(this one no longer exist in i_video.c
file), and header file contains:
extern const pixel_t I_BlendOver (const pixel_t bg, const pixel_t fg, const int amount);
#define I_BlendOverTranmap(bg, fg) I_BlendOver(bg, fg, 0xa8)
Compiler gives following multiple errors like:
# cmake --build build
[5/47] Building C object src/doom/CMakeFiles/doom.dir/r_things.c.obj
FAILED: src/doom/CMakeFiles/doom.dir/r_things.c.obj
R:\MSYS\mingw64\bin\cc.exe -IR:/MSYS/home/julia/crispy-doom/src/doom/.. -IR:/MSYS/home/julia/crispy-doom/build/src/doom/../.. -isystem R:/MSYS/mingw64/include/SDL2 -O3 -DNDEBUG -Wall -Wdeclaration-after-statement -Wredundant-decls -MD -MT src/doom/CMakeFiles/doom.dir/r_things.c.obj -MF src\doom\CMakeFiles\doom.dir\r_things.c.obj.d -o src/doom/CMakeFiles/doom.dir/r_things.c.obj -c R:/MSYS/home/julia/crispy-doom/src/doom/r_things.c
R:/MSYS/home/julia/crispy-doom/src/doom/r_things.c: In function 'R_DrawVisSprite':
R:/MSYS/home/julia/crispy-doom/src/doom/r_things.c:547:17: error: 'I_BlendOverTranmap' undeclared (first use in this function); did you mean 'I_BlendOverXlatab'?
547 | blendfunc = I_BlendOverTranmap;
| ^~~~~~~~~~~~~~~~~~
| I_BlendOverXlatab
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, dammit, function pointers. Right this won't work. The solution you suggested before looked reasonable, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
src/strife/f_wipe.c
Outdated
//wipe_scr = I_VideoBuffer; | ||
#else | ||
// [crispy] In TrueColor render perform everything in common buffer. | ||
// Otherwise serious malloc errors will happen. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really, even if you allocate the correct amount of memory with sizeof(*wipe_scr)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'm afraid. "Debug" build type can survive it, but "Release" type almost imideatelly crashes after crossfade or next hitting of Z_Malloc
(like opening Load menu) with something like this:
Thread 1 received signal SIGSEGV, Segmentation fault.
0x00007ff6f69f396e in Z_Malloc ()
(gdb) bt
#0 0x00007ff6f69f396e in Z_Malloc ()
#1 0x00007ff6f6a07357 in M_StringAlloc.constprop.0 ()
#2 0x00007ff6f6a07c74 in M_SafeFilePath ()
#3 0x00007ff6f6a026f5 in M_ReadSaveStrings ()
#4 0x00007ff6f6a06415 in M_Responder ()
#5 0x00007ff6f69f698d in D_ProcessEvents ()
#6 0x00007ff6f69c6930 in BuildNewTic ()
#7 0x00007ff6f69c6afc in NetUpdate.part.0 ()
#8 0x00007ff6f69c7782 in TryRunTics ()
#9 0x00007ff6f69f6e9a in D_DoomLoop ()
#10 0x00007ff6f69f89c7 in D_DoomMain ()
#11 0x00007ff6f69c159f in SDL_main ()
#12 0x00007ff6f6a3f0fe in main_getcmdline ()
#13 0x00007ff6f69c12ee in __tmainCRTStartup () at C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:267
#14 0x00007ff6f69c13e6 in WinMainCRTStartup () at C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:157
(gdb)
Since it's not a simple game of PU_STATIC
vs. PU_CACHE
, using of I_VideoBuffer
is the best I can do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to have a closer look at this, probably this weekend, before I wave it through.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works for me:
--- a/src/strife/f_wipe.c
+++ b/src/strife/f_wipe.c
@@ -283,14 +283,8 @@ wipe_ScreenWipe
{
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
+ wipe_scr = (pixel_t *) Z_Malloc(width*height*sizeof(*wipe_scr), PU_STATIC, 0); // DEBUG
//wipe_scr = I_VideoBuffer;
-#else
- // [crispy] In TrueColor render perform everything in common buffer.
- // Otherwise serious malloc errors will happen.
- wipe_scr = I_VideoBuffer;
-#endif
(*wipes[wipeno*3])(width, height, ticks);
}
@@ -298,10 +292,8 @@ wipe_ScreenWipe
V_MarkRect(0, 0, width, height);
rc = (*wipes[wipeno*3+1])(width, height, ticks);
-#ifndef CRISPY_TRUECOLOR
// haleyjd 20110629 [STRIFE]: Copy temp buffer to the real screen.
V_DrawBlock(x, y, width, height, wipe_scr);
-#endif
// final stuff
if (rc)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I have missed most important part just like in other parts of wiping functions - *sizeof(*wipe_scr)
. Working on my side too, thank you! Will commit changes soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say let's go the Doom way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But for some reason there is this comment:
// haleyjd 20110629 [STRIFE]: We *must* use a temp buffer here.
So, better explicitly call Z_Free()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call Z_Free(wipe_scr); below this line.
Hm, didn't worked for me, that's odd. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, Z_Free()
only marks a block in zone memory as "free". That is, Z_Malloc()
only allocates in the zone memory as well, so why does the heap memory consumption of the whole process grow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You right, turns out, it's not a wipe, it's something else. Even if wipe is disabled here as:
// save the current screen if about to wipe
if (gamestate != wipegamestate && false) // +++ && false
...leak still happening. Probably something around with malloc
?
Guess it's all done and ready for full review. Small testing map for checking translucency + translation effects: 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 bandicam.2024-04-14.12-10-35-265.mp4 |
Co-Authored-By: Fabian Greffrath <fabian@greffrath.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job, thank you!
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. |
🤩 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? |
Never mind, I think we will only have to remove the first sentence. 😉
I'd like to announce this as part of the next "official" release. People closely following the project will notice earliert anyway. |
Done! ⚡️
Ah, sure, whatever you say. |
Small and brain-dead trick for smoother crossfading without post-effect delay: +++ 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
+++ 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 Another notable thing is - if using |
This is the final chapter of Crispy true color story, what else can be said? 🙂 Just a few remarks and TODOs:
TODO: Screen wiping disabled. It is not possible to useXLATAB
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.TODOs : Graphical startup with RAW patches working, but not sure it's an optimal way, sincepal_color[]
must be filled before graphical startup, and then filled again after generating colormaps.Brightmaps working. Strife is usingCOLORMAP
for such trick, it was easy it replicate it by locking necessary colors on the first light level.Strife using two levels of translucency: 25% and 75% for column/segs drawing and only 75% for patch drawing. Kaiser's comment in r_segs.c helped a lot in the question of opacity values.And of course, small and not very colorful example of before and after: