Skip to content
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

Better support for mirrors seeing other mirrors #646

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DanielGibson
Copy link
Member

It used to crash because after recursing into R_MirrorPoint(), which calls tr.CropRenderSize(), about MAX_RENDER_CROPS times, CropRenderSize() wrote behind the end of tr.renderCrops[] and damaged a pointer.
Even if it didn't crash because of that, endlessly recursing is a bad idea, so it's limited now. The limit can be configued with the new r_maxMirrorRecursion CVar, but the hard limit is MAX_RENDER_CROPS-3 = 5

Another problem was that mirrors and other subviews render into a texture, specifically globalImages->scratchImage ("_scratch"), so it could happen that it got written to several times before being used. Now there are 8 scratch images (+ the original "_scratch" which is also used by gamecode) that the subview code cycles through to minimize the chance of overwriting something that's still in use.

This fixes #367

It used to crash because after recursing into R_MirrorPoint(), which
calls tr.CropRenderSize(), about MAX_RENDER_CROPS times,
CropRenderSize() wrote behind the end of tr.renderCrops[] and damaged
a pointer.
Even if it didn't crash because of that, endlessly recursing is a bad
idea, so it's limited now. The limit can be configued with the new
r_maxMirrorRecursion CVar, but the hard limit is MAX_RENDER_CROPS-3 = 5

Another problem was that mirrors and other subviews render into
a texture, specifically globalImages->scratchImage ("_scratch"), so
it could happen that it got written to several times before being used.
Now there are 8 scratch images (+ the original "_scratch" which is also
used by gamecode) that the subview code cycles through to minimize the
chance of overwriting something that's still in use.
@DanielGibson
Copy link
Member Author

DanielGibson commented Jan 23, 2025

As usual, download testbuilds at https://github.com/dhewm/dhewm3/actions/runs/12921127187?pr=646 under "Artifacts" (once the builds are done)

Video:

d3-mirrors-fixed.mp4

@Sol1vaN
Copy link

Sol1vaN commented Jan 25, 2025

I was just about to make a request about this.

A long time ago, I requested the same thing for Doom 3 on an old forum, and someone replied suggesting creating the following script:

//reflect stage
	{
		mirrorRenderMap 512 512		//detail of reflection, higher # = better
		blend blend
		alpha 0.3			//lower # = more transparent
		translate 0.5, 0.5
		scale 0.5, 0.5
	}
}

However, this have the same issues that you report early.
Your code are much better, but still have some glitches sadly.

I don't know why, but on Doom 3 Alpha Leaked (from 2002) this feature works awesome, look:
shot0032
shot0034

the crystal visors from soldiers helmet's have reflect effect too:
shot0018
shot0027
shot0028

reflect glass window showcase at 15:13
https://youtu.be/Ok2rPUibnZw

doom 3 retail reflect glass script:
reflectglass.pk4.zip

we can compare the alpha script, are very similar:

textures/sfx/techglassadd
{
	translucent
	twosided
	noshadows
	qer_editorimage	textures/sfx/techglass1.tga
	{
		blend 	add
		cubeMap	env/sk7
		texgen	reflect
	}
	{
		blend		add
		map 		textures/sfx/techglass1_d.tga
		scale		8 , 8
	}
}

doom 3 alpha leaked script:
doom 3 alpha leaked reflect glass.zip

I hope you can do it, doom 3 alpha leaked looks awesome and Doom 3 had much of those effects stripped away.

@Sol1vaN
Copy link

Sol1vaN commented Jan 25, 2025

ou! i noticed that #367 was maded by me on 2021 ! 😨
haha! I completely forgot about it 🤣
and thanks Daniel for making this happen

@Sol1vaN
Copy link

Sol1vaN commented Jan 25, 2025

Testing time...

Render into the same image problem
on Delta2a the glasses shows weird reflexes, seems caused by "R_MirrorRender() always renders into the same image"
https://github.com/user-attachments/assets/8be1e38c-6c1b-4d39-9361-cbb5afa58015

Ghosting problem
I think (im not sure), because now we have "there are 8 scratch images" the engine do a ghost reflexes by the 8 scratch images?, look:
https://github.com/user-attachments/assets/901ce173-0cff-4751-b0b3-58146bc761e1
https://github.com/user-attachments/assets/05986c8a-6a9e-4351-9632-1d15cb9ac708

If you pay atention at the background of reflected image, theres a "ghosting" effect.
For me is fine, i like it, but is there.

I hope this help some.

@DanielGibson
Copy link
Member Author

Something I noticed: There are two ways to create mirrors in Doom3.

One is the global mirror material keyword - that's what the mirror in the bathroom and also textures/common/mirror use.
This does not go through the code I changed and seems to support recursion to some degree and it also seems to work if the mirrors are not axis-aligned, however when you see the mirror you're currently looking into in the other mirror, it appears black:

shot00138

The other way to create a mirror is the mirrorRenderMap material stage keyword. That's what those glass shaders provided in #367 use, and that's what seems to be glitchy.

Note that I did not try to make the mirror using mirror two-sided, who knows what that would cause..

@Sol1vaN
Copy link

Sol1vaN commented Jan 26, 2025

hm, and what about making [mirrorRenderMap] only reflects objets? i mean, i noticed on alpha leaked only the player and monster are reflected, not the world map.
the world details on glasses are managed by those cubemaps or whatever doom3 use for it, we don't care about reflect the entire world map, i think.

it will be a solution for solve these weird glitch that shows portions of the world map into the reflex.
what do you think? is feasible do that?

@DanielGibson
Copy link
Member Author

DanielGibson commented Jan 26, 2025

hm, and what about making [mirrorRenderMap] only reflects objets? i mean, i noticed on alpha leaked only player and monster are reflected, not the world map.

Not sure this is possible, AFAIK a feature of Doom3 (compared to previous idTech versions) was to render everything the same ("unified rendering"). Also, it wouldn't help with glitches involving the playermodel like in this video:

d3-mirrorbug.mp4

Generally speaking, rendering only objects and using a cubemap for the world might be a nice optimization for rendering mirrors and especially mirror-like surfaces that don't need look perfect like glass. No idea if this is something that is or was commonly done though.
And there are probably problems like when there's a wall in the world (and thus the cubemap) that hides objects that the mirror otherwise would show (esp. relevant for objects partially behind the wall) - though maybe that could be worked around with depth information in the cubemap? Is that a thing? No idea, I'm not a graphics programmer and don't really want to be one..

Anyway, I think at least part of the problem with the mirrorRenderMap glitches is that:

  • R_MirrorRender() sets the scratch image (which now thanks to my changes should not be reused too soon) to stage->image
  • where stage is a textureStage_t which is set in R_GenerateSurfaceSubview() as const_cast<textureStage_t *>(&stage->texture)
  • and in R_GenerateSurfaceSubview() stage is const shaderStage_t *stage = shader->GetStage( i );
  • and shader is a const idMaterial* set from drawSurf->material

The problem here is that the idMaterial is a global thing representing the

textures/glass/glass1
{
	noSelfShadow
	noshadows
	...

thing from the .mtr file.
This means that per material specified in the .mtr there is only one idMaterial, which has only one shaderStage_t per stage, which has only one textureStage_t, which has only one idImage* image that R_MirrorRender() can assign the scrap image to.
(That scrap image is the on-GPU texture the subview is rendered to, so that texture can then be used on the window and be rendered half-translucent)

So while that image for the textureStage_t is rendered byR_MirrorRender() for one window, it will eventually recurse into R_MirrorRender() with the same textureStage_t and the stage->image pointer will be overwritten with another scratch image for the recursed view from another window.

(I tried setting one of the windows in my testmap to textures/glass/glass2 but that didn't seem to help - unsure why, maybe identical textureStage_t are shared between multiple shaders? Or maybe there are other/additional things going wrong? IDK also, for some reason that's also unclear to me, glass2 only reflected the player on one side - the one that "looks" into the direction of the other windows. It's all broken)

my testmap:
mymirrortestmap.zip

the used materials are from reflectglass.zip from #367

@Sol1vaN
Copy link

Sol1vaN commented Jan 27, 2025

hm, now i understand why mirrors and reflexes are a nightmare to program on video games.
PiccuEngine has similar problem with this, the reflex that you see when look a wall mirror trough the rearview mirror of the ship is the front side of the ship, not the back side.

In the video can be more detailed what I'm say:
https://youtu.be/ItUhNAq5PuI

Is very sad, but the old engines manage this problem and I don't know how.
Like Duke Nukem 3D 🤔

@DanielGibson
Copy link
Member Author

Duke3D supported mirrors looking into mirrors?

@Sol1vaN
Copy link

Sol1vaN commented Jan 27, 2025

Duke3D supported mirrors looking into mirrors?

youre right 😂 sorry, forget those what i say

@DanielGibson
Copy link
Member Author

Also note that Duke3Ds mirrors were really hacky: They required an empty room on the other side of the mirror that's big enough to hold everything the mirror can show, the engine then automatically mirrored the geometry into there.

In Doom3 mirrors can be in the middle of the room and you can walk around them if you want.

@Sol1vaN
Copy link

Sol1vaN commented Jan 27, 2025

wow!! i don't know about that of doom 3! 😮😮😮 i'll try.
also this remember me Mario 64 that do some similar, copying the same geometry inverted.

@Sol1vaN
Copy link

Sol1vaN commented Jan 31, 2025

some ideas, im not programmer, so i think may be wrong, but here we go:

Idea 1: Assign a Single Scratch Image per Surface.
The most straightforward solution is to make sure that each reflective surface has its own temporary image instead of sharing it with other surfaces.

Modify R_MirrorRender() in tr_subview.cpp to create a new texture on each render

void R_MirrorRender( drawSurf_t *surf, textureStage_t *stage, idScreenRect scissor ) {
    viewDef_t *parms;

    if ( stage->dynamicFrameCount == tr.frameCount ) {
        return;
    }

    parms = R_MirrorViewBySurface( surf );
    if ( !parms ) {
        return;
    }

    tr.CropRenderSize( stage->width, stage->height, true );

    parms->renderView.x = 0;
    parms->renderView.y = 0;
    parms->renderView.width = SCREEN_WIDTH;
    parms->renderView.height = SCREEN_HEIGHT;

    tr.RenderViewToViewport( &parms->renderView, &parms->viewport );

    parms->scissor.x1 = 0;
    parms->scissor.y1 = 0;
    parms->scissor.x2 = parms->viewport.x2 - parms->viewport.x1;
    parms->scissor.y2 = parms->viewport.y2 - parms->viewport.y1;

    parms->superView = tr.viewDef;
    parms->subviewSurface = surf;

    parms->isMirror = ( ( (int)parms->isMirror ^ (int)tr.viewDef->isMirror ) != 0 );

    R_RenderView( parms );

    stage->dynamicFrameCount = tr.frameCount;

    // Asignar una nueva textura en lugar de reutilizar la scratch image global
    stage->image = globalImages->AllocImage( "mirrorSurface" );
    
    tr.CaptureRenderToImage( stage->image->imgName );
    tr.UnCrop();
}

What I think this do:

A new texture is created in globalImages->AllocImage("mirrorSurface") instead of using GetNextScratchImage().
This maybe prevents multiple surfaces from overwriting the same mirror image.

Idea 2: Force Different Materials for Each Surface

Duplicate the material but in the .mtr file
Another less intrusive solution would be to duplicate the material in the .mtr file for each reflective surface, ensuring that each has its own idMaterial.

textures/glass/glass1
{
    noSelfShadow
    noshadows
    {
        blend blend
        mirrorRenderMap
        alpha 0.3
    }
}

textures/glass/glass2
{
    noSelfShadow
    noshadows
    {
        blend blend
        mirrorRenderMap
        alpha 0.3
    }
}

This would cause glass1 and glass2 to be treated as separate materials and not share the same reflection texture, reducing the overwriting issue.

Idea 3: Use Framebuffers Instead of Scratch Textures

modify R_MirrorRender() to render in a framebuffer
Or... may be possible to modify R_MirrorRender() to render the reflection into a separate framebuffer??? and then take the result from there instead of a scratch image???

but this i think this last idea is a very very more hard work to do. i don't know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Reflect Glasses effect problem.
2 participants