-
Notifications
You must be signed in to change notification settings - Fork 133
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
Heretic: add true color support #1111
Conversation
Despite of here in V_ functions TINTTAB is used for shadows drawing, it's initial purpose is slightly different - it is a blending table, which is *also* used for shadows. "drawtinttab" must be more clear in this context, and in case one day TC implementation will be done for Strife, XLA's function should be named "drawxlatab".
Remove duplication of invul. colormap generation.
…olormap handling Raven Software's oversight, or attempt to reduce memory consumption? https://github.com/OpenSourcedGames/Heretic/blob/master/Heretic%20Source/R_MAIN.C#L776
As not used in true color mode at all.
Fairly simple. We need only proper 256 colors for drawing E2END itself, but we also need to do a proper reset while switching back to normal palette, otherwise we'll get some wrong colors. Suggestions for future: - externalize gamma2table to i_video.h - externalize R_InitColormaps and R_SetUnderwaterPalette to r_local.h
Also, "viewactive" is not needed. Despite of existing ovet the code, it's not doing anything. Using "setsizeneeded" wasn't optimal, it too expensive. All we need is: - BorderNeedRefresh - updates screen border and bezel, including message area. - SB_state - do a full redraw of whole status bar.
This is, hopefully, as small diff as possible, which could be useful for custom COLORMAPs (but are there any existing?). Note that unlike Doom, REINDEX is using lesser color range to be as close to original smoothing as possible. At this point, only background is not working on automap.
I've just had a look it this so far and it is indeed trickier than it initially seemed. The code picks pixels that are already drawn to the framebuffer and feeds them back into the colormaps array to get an increasingly darker shaded pixel back. This won't work with RGB pixels, of course, so we'll have to e.g. introduce another function that takes a pixel and a "darkening value" and returns a new pixel value. |
It has been some time since I've looked at the automap background parallaxing but I'm wondering if it could be refactored to use Question: Does the end-of-game demonscroll sequence work okay in true color? |
Hmph. Maybe we can apply something like Non fair but extremely simple approach is to draw gradients by hand, hard-code them into executable (such gradients aren't supposed to be replaced by mods any way) and draw them as translucent patches. But this is artistical approach, sadly.
Absolutely fine, as well as all other RAW screens. Episode 2 underwater ending with custom palette working fine, too. |
But even if w/o parallax effect, how to properly draw tiled 320x158 raw lump? Should be something like:
But it's not, it appears as broken. 🙁 |
Needs polish, but something like this should do the trick: Diffdiff --git a/src/heretic/am_map.c b/src/heretic/am_map.c
index 32a3f7b5..53e6b0b0 100644
--- a/src/heretic/am_map.c
+++ b/src/heretic/am_map.c
@@ -1044,10 +1044,8 @@ void AM_Ticker(void)
void AM_clearFB(int color)
{
- int i, j;
int dmapx;
int dmapy;
- int x1, x2, x3;
if (followplayer && !paused)
{
@@ -1106,31 +1104,10 @@ void AM_clearFB(int color)
// [crispy] To support widescreen, increase the number of possible
// background tiles from 2 to 3. To support rendering at 2x resolution,
// treat original 320 x 158 tile image as 640 x 79.
- j = mapystart * (MAPBGROUNDWIDTH << crispy->hires);
- x1 = mapxstart;
- x2 = finit_width - x1;
-
- if (x2 > MAPBGROUNDWIDTH << crispy->hires)
- x2 = MAPBGROUNDWIDTH << crispy->hires;
-
- x3 = finit_width - x2 - x1;
-
- for (i = 0; i < finit_height; i++)
- {
- memcpy(I_VideoBuffer + i * finit_width,
- maplump + j + (MAPBGROUNDWIDTH << crispy->hires) - x3, x3);
-
- memcpy(I_VideoBuffer + i * finit_width + x3,
- maplump + j + (MAPBGROUNDWIDTH << crispy->hires) - x2, x2);
-
- memcpy(I_VideoBuffer + i * finit_width + x2 + x3,
- maplump + j, x1);
-
- j += MAPBGROUNDWIDTH << crispy->hires;
- if (j >= MAPBGROUNDHEIGHT * MAPBGROUNDWIDTH)
- j = 0;
- }
+ V_DrawRawTiled(mapxstart, mapystart, MAPBGROUNDWIDTH << crispy->hires,
+ MAPBGROUNDHEIGHT >> crispy->hires,
+ SCREENHEIGHT - SBARHEIGHT, maplump);
// memcpy(I_VideoBuffer, maplump, finit_width*finit_height);
// memset(fb, color, f_w*f_h);
diff --git a/src/v_video.c b/src/v_video.c
index f602feb0..c7bfd65c 100644
--- a/src/v_video.c
+++ b/src/v_video.c
@@ -732,6 +732,27 @@ void V_LoadXlaTable(void)
xlatab = W_CacheLumpName("XLATAB", PU_STATIC);
}
+void V_DrawRawTiled(int x_off, int y_off, int width, int height, int v_max,
+ byte *src)
+{
+ pixel_t *dest = dest_screen;
+ int x, y;
+
+ for (int i = 0; i < v_max; i++)
+ {
+ for (int j = 0; j < SCREENWIDTH; j++)
+ {
+ x = (j + x_off) % width;
+ y = (i + y_off) % height;
+#ifndef CRISPY_TRUECOLOR
+ *dest++ = src[width * y + x];
+#else
+ *dest++ = colormaps[src[width * y + x]];
+#endif
+ }
+ }
+}
+
//
// V_DrawBlock
// Draw a linear block of pixels into the view buffer.
diff --git a/src/v_video.h b/src/v_video.h
index ee13a51b..437673fa 100644
--- a/src/v_video.h
+++ b/src/v_video.h
@@ -67,6 +67,9 @@ void V_DrawXlaPatch(int x, int y, patch_t * patch); // villsa [STRIFE]
void V_DrawPatchDirect(int x, int y, patch_t *patch);
void V_DrawPatchFullScreen(patch_t *patch, boolean flipped);
+void V_DrawRawTiled(int x_off, int y_off, int width, int height, int v_max,
+ byte *src);
+
// Draw a linear block of pixels into the view buffer.
void V_DrawBlock(int x, int y, int width, int height, pixel_t *src);
I always hated that |
These two commits are pure Black Magic, I even willing to ask about them! 🧙 Looks likes everything is done and working fine, and can be marked as review-ready for now. But before, couple of suggested self-nitpicks:
|
src/heretic/r_data.c
Outdated
// [crispy] Changes palette to E2PAL. Used exclusively in true color rendering | ||
// for proper drawing of E2END pic in F_DrawUnderwater. Changing palette back to | ||
// original PLAYPAL for restoring proper colors will be done in R_InitColormaps. | ||
void R_SetUnderwaterPalette(void) |
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 function would be more universally usable (for truecolor Hexen, maybe) if you passed the palette lump name as an argument.
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.
Fixed in 00c44aa. It's still local as discussed, but passing parametrized lump name was a very good idea:
- This makes code much neater and w/o extra macroses.
E2PAL
seems to be HeHacked`able name, so we have to care about it's possible name replaced, even though such replacement probably will never happen.
src/heretic/r_main.c
Outdated
@@ -879,7 +879,9 @@ void R_SetupFrame(player_t * player) | |||
if (player->fixedcolormap) | |||
{ | |||
fixedcolormap = colormaps + player->fixedcolormap | |||
* 256 * sizeof(lighttable_t); | |||
// [crispy] sizeof(lighttable_t) not needed in paletted render |
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.
Just get away with it, not worth a comment.
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.
Fixed in 1f81552. But should I remove /* * sizeof(lighttable_t)*/
as well? A bit scarry for possible diff in future, like in Chocolate Doom sizeof
exist, but here it doesn't. The reason might be not obivious for someone who haven't suffered this issue.
Thank you! 😁
Yes, let's please not add even more
Let's fix it while we are at it.
Yes, please. These are really annoying code duplications.
I think you even obsoleted these arrays entirely by using the |
Oops, "not willing to ask" was meant! 😱
I have double-checked, Heretic is the only game which is using such palette swapping, Hexen and Strife are operating with
Should I replace true color only code, or paletted code as well? And, in Heretic only, or in all four games? The rest of comments are clear, thanks for making a fresh look! |
I guessed that. 😉
Alright to leave it in place then. The function name is fine.
All games, all modes, please. |
Well, but that's really stuff for a separate PR. |
Not sure if R_SetUnderwaterPalette needed to be guarded by a macro in header file, but probably this will make sence for better understanding about the nature of this function, i.e. it's "for true color only".
No changes for Hexen and Strife needed, as these externals were made for true color render only.
Thinking more, you're right, Renaming |
Much neater and less macrocized now. Thanks, Fabian!
Just checked, fortunately not, still both arrays are used, except only first their numbers are needed. We still must have two arrays for normal and overlay modes, but now they can be as simple as:
Fun thing is - the only different color is the first one now (96 vs 100), it's common walls. Easiest way here is to use single array with middle color between |
* Revert back pixel_t to byte for arrays of line colors. My typo made back in the time I was just started working on automap. * Still use two arrays for normal and overlay mode. Idea of having "middle" color for walls drawing and array does not seems to be good enough, as we loose original color for both modes. In theory, both arrays can be consolidated into one, but this is a bit tricky because we have to define "which color should be used for common walls drawing" w/o hitting if/else condition.
I think it's all done and ready for full review. My last idea is reducing amount of incoming parateters for DrawWuLine, since |
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.
Awesome job, thank you very much!
It was a pleasure! And it's started from pure interest, thank you for such gift as true color render! On next step I'll try to simplify flat filling, I think function should accept different heights for filling both full screens (height is always Meanwhile, which game should be adopted for true color next? We have Hexen and Strife left, both are evil, here's why: Hexen:
Strife:
|
@mikeday0 Oops, sorry! I just realized that I outright ignored your code suggestion. |
No need to apologize! Just happy to see this truecolor effort come to fruition. |
Pretty much same to Doom, except much more strict with translucency effects - we must keep original look & feel as much as possible, so there is no additive blending.
I wasn't able to resolve two small remaining problems:
byte
topixel_t
doesn't work. Something must be wrong with eitherdest =
declaring, or insidewhile
loop.maplump
is a 320x158 raw screen, we can't usememcpy
here. In theory, we can draw it same way as common 64x64 flat tiles, but how to achieve that dang parallax effect then - no idea.Any help is highly appreciated! ❤️