View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0006434 | The Dark Mod | Graphics | public | 06.01.2024 16:31 | 14.04.2024 02:07 |
Reporter | stgatilov | Assigned To | stgatilov | ||
Priority | normal | Severity | feature | Reproducibility | sometimes |
Status | feedback | Resolution | open | ||
Product Version | TDM 2.12 | ||||
Target Version | TDM 2.13 | ||||
Summary | 0006434: Try to fully support nested/recursive subviews | ||||
Description | I have reviewed the subviews code, and it seems that there is no inherent problem in having arbitrary nested subviews. I mean the cases like: 1) Remote camera looking at mirror, or mirror reflecting remote screen. 2) Remote camera + mirror cases combined with skybox. 3) Several mirrors on the same scene. 4) Several mirrors showing reflection of each other, possible infinitely (should be capped be engine). Indeed, such effects would cost a lot of performance. But with hard limits like we have now, mappers can fall into one of these cases accidentally and get broken rendering. | ||||
Steps To Reproduce | TODO: Need an ambitious test map for this. | ||||
Tags | No tags attached. | ||||
After looking at 0006424, it seems that the only problem right now is using texture of hardcoded name as output for mirror / remote subviews. Instead, we can leave output texture name undefined in the material file (e.g. "$auto$"). Then the name of the texture should be generated from recursion path of subviews to keep them distinct. Note that several mirrors in the same scene should get different names too. The challenge is to ensure that this auto-generated name is used in this material in the parent view. Note that there are nasty exceptions. For instance, skybox does not copy output to texture, but simply leaves itself as background. This, there can be only one skybox and it must be rendered last (among all subviews of the same view). |
|
Added test/6434_nested_subviews.map for testing. | |
Some WIP attached. I created a storage of "subview output images" in RenderSystem. Whenever subview is generated, an image is taken out of this storage of matching size (there there is none, then a new one is generated). The copy command is done into this image, and then it is attached to drawSurf_t to override material's texture (works only inside "simple" surface pass, just like e.g. cinematics). If some image in the storage is not used for 100 frames, then it is garbage-collected. There are various blockers for recursing into mirrors, for recursing into same surface, etc. I only tested on remote renders and mirrors, so I deleted all blockers from there. There is still a sanity blocker by recursion depth (output texture is _black if blocked), and maybe we should add more sanity blockers for performance reasons. I noticed some issues with mirror. Right now it sets a clip plane, otherwise objects slightly behind mirror will mess up everything. But this clip plane is only taken into account in depth stage, although I see problems at least with stencil shadows too. Besides, I think we need several clip planes if we want to support mirror inside mirror. The skybox is broken inside both remote renders and mirrors. I think its logic simple does not support any kind of subviews --- should be fixed there. Finally, I did not test X-ray. Right now it is handled separately for some reason despite it copying the output into texture. Perhaps I need to refactor it to works similar to remote renders. P.S. And thief looks black in all subviews, which is also perhaps a bug. P.P.S. Restoring a possibility to render mirrored view in lower resolution would be nice for performance too. 6434_nested_subviews_wip.patch (10,510 bytes)
Index: renderer/backend/stages/SurfacePassesStage.cpp =================================================================== --- renderer/backend/stages/SurfacePassesStage.cpp (revision 10623) +++ renderer/backend/stages/SurfacePassesStage.cpp (working copy) @@ -334,7 +334,7 @@ // bind texture (either 2D texture or cubemap is used) GL_SelectTexture( 0 ); - BindVariableStageImage( &pStage->texture, regs ); + BindVariableStageImage( drawSurf, &pStage->texture, regs ); if ( texgen == TEXGEN_CUBEMAP ) { uniforms->cubemap.Set( 0 ); uniforms->texture.Set( 1 ); @@ -549,7 +549,7 @@ RB_DrawElementsWithCounters( drawSurf, DCK_SURFACE_PASS ); } -void SurfacePassesStage::BindVariableStageImage( const textureStage_t *texture, const float *regs ) { +void SurfacePassesStage::BindVariableStageImage( const drawSurf_t *drawSurf, const textureStage_t *texture, const float *regs ) { if ( texture->cinematic ) { if ( r_skipDynamicTextures.GetBool() ) { globalImages->defaultImage->Bind(); @@ -568,5 +568,8 @@ } } else if ( texture->image ) { texture->image->Bind(); + } else if ( texture->dynamic && drawSurf->dynamicImageOverride ) { + // stgatilov #6434: image generated by subview + drawSurf->dynamicImageOverride->Bind(); } } Index: renderer/backend/stages/SurfacePassesStage.h =================================================================== --- renderer/backend/stages/SurfacePassesStage.h (revision 10623) +++ renderer/backend/stages/SurfacePassesStage.h (working copy) @@ -54,5 +54,5 @@ void DrawSoftParticle( const drawSurf_t *drawSurf, const shaderStage_t *pStage ); void DrawCustomShader( const drawSurf_t *drawSurf, const shaderStage_t *pStage ); - void BindVariableStageImage( const textureStage_t *texture, const float *regs ); + void BindVariableStageImage( const drawSurf_t *drawSurf, const textureStage_t *texture, const float *regs ); }; Index: renderer/frontend/tr_light.cpp =================================================================== --- renderer/frontend/tr_light.cpp (revision 10623) +++ renderer/frontend/tr_light.cpp (working copy) @@ -1269,6 +1269,7 @@ drawSurf->CopyGeo( tri ); drawSurf->space = space; drawSurf->material = material; + drawSurf->dynamicImageOverride = nullptr; drawSurf->scissorRect = scissor; drawSurf->sort = material->GetSort(); drawSurf->dsFlags = 0; Index: renderer/frontend/tr_subview.cpp =================================================================== --- renderer/frontend/tr_subview.cpp (revision 10623) +++ renderer/frontend/tr_subview.cpp (working copy) @@ -19,12 +19,14 @@ #include "renderer/tr_local.h" #include "game/Grabber.h" + +static const int MAX_SUBVIEW_DEPTH = 4; + typedef struct { idVec3 origin; idMat3 axis; } orientation_t; - /* ================= R_MirrorPoint @@ -274,9 +276,9 @@ */ static void R_RemoteRender( drawSurf_t *surf, textureStage_t *stage ) { - // remote views can be reused in a single frame +/* // remote views can be reused in a single frame if ( stage->dynamicFrameCount == tr.frameCount ) - return; + return;*/ // if the entity doesn't have a remoteRenderView, do nothing if ( !surf->space->entityDef->parms.remoteRenderView ) @@ -288,6 +290,8 @@ parms->isSubview = true; parms->isMirror = false; + // if we see remote screen in mirror, drop mirror's clip plane + parms->clipPlane = nullptr; parms->renderView = *surf->space->entityDef->parms.remoteRenderView; parms->renderView.viewID = VID_SUBVIEW; // clear to allow player bodies to show up, and suppress view weapons @@ -314,11 +318,17 @@ R_RenderView(*parms); // copy this rendering to the image - stage->dynamicFrameCount = tr.frameCount; + /*stage->dynamicFrameCount = tr.frameCount; if ( !stage->image ) stage->image = globalImages->scratchImage; - tr.CaptureRenderToImage( *stage->image->AsScratch() ); + tr.CaptureRenderToImage( *stage->image->AsScratch() );*/ + + stage->image = nullptr; + idImageScratch *outputTexture = tr.CreateImageForSubview(); + tr.CaptureRenderToImage( *outputTexture ); + surf->dynamicImageOverride = outputTexture; + tr.UnCrop(); } @@ -330,13 +340,13 @@ void R_MirrorRender( drawSurf_t *surf, textureStage_t *stage, idScreenRect& scissor ) { viewDef_t *parms; - if ( tr.viewDef->superView && tr.viewDef->superView->isSubview ) // #4615 HOM effect - only draw mirrors from player's view and top-level subviews + /*if ( tr.viewDef->superView && tr.viewDef->superView->isSubview ) // #4615 HOM effect - only draw mirrors from player's view and top-level subviews return; // remote views can be reused in a single frame if ( stage->dynamicFrameCount == tr.frameCount ) { return; - } + }*/ // issue a new view command parms = R_MirrorViewBySurface( surf ); @@ -365,11 +375,17 @@ R_RenderView( *parms ); // copy this rendering to the image - stage->dynamicFrameCount = tr.frameCount; + /*stage->dynamicFrameCount = tr.frameCount; if ( !stage->image ) stage->image = globalImages->scratchImage; - tr.CaptureRenderToImage( *stage->image->AsScratch() ); + tr.CaptureRenderToImage( *stage->image->AsScratch() );*/ + + stage->image = nullptr; + idImageScratch *outputTexture = tr.CreateImageForSubview(); + tr.CaptureRenderToImage( *outputTexture ); + surf->dynamicImageOverride = outputTexture; + //tr.UnCrop(); } @@ -678,7 +694,6 @@ */ bool R_GenerateSurfaceSubview( drawSurf_t *drawSurf ) { idBounds ndcBounds; - viewDef_t *parms; const idMaterial *shader; // for testing the performance hit @@ -690,7 +705,7 @@ shader = drawSurf->material; - // never recurse through a subview surface that we are + /*// never recurse through a subview surface that we are // already seeing through for ( parms = tr.viewDef ; parms ; parms = parms->superView ) { if ( parms->subviewSurface @@ -700,6 +715,12 @@ } } if ( parms ) + return false;*/ + + int depth = 0; + for (viewDef_s *view = tr.viewDef; view; view = view->superView) + depth++; + if (depth > MAX_SUBVIEW_DEPTH) return false; // crop the scissor bounds based on the precise cull @@ -787,10 +808,14 @@ if ( !shader || !shader->HasSubview() ) continue; - if ( shader->GetSort() != SS_PORTAL_SKY ) // portal sky needs to be the last one, and only once + if ( shader->GetSort() != SS_PORTAL_SKY ) {// portal sky needs to be the last one, and only once if ( R_GenerateSurfaceSubview( drawSurf ) ) { subviews = true; + } else { + // #6434: probably blocked due to limits: render as black image + drawSurf->dynamicImageOverride = globalImages->blackImage; } + } } static bool dontReenter = false; Index: renderer/RenderSystem.cpp =================================================================== --- renderer/RenderSystem.cpp (revision 10623) +++ renderer/RenderSystem.cpp (working copy) @@ -702,6 +702,8 @@ // we can now release the vertexes used this frame vertexCache.EndFrame(); + FreeOldSubviewImages(); + if ( session->writeDemo ) { session->writeDemo->WriteInt( DS_RENDER ); session->writeDemo->WriteInt( DC_END_FRAME ); @@ -741,6 +743,70 @@ viewport.y2 = idMath::FtoiTrunc( ( rc.y + rc.height ) - floor( renderView.y * hRatio + 0.5f ) - 1 ); } + +void idRenderSystemLocal::FreeOldSubviewImages() { + // this method should be called outside parallel section + assert(!session->IsFrontend()); + + static const int SUBVIEW_IMAGE_KILL_AFTER_FRAMES = 100; + + for (int i = 0; i < subviewImages.Num(); i++) { + ImageForSubview &si = subviewImages[i]; + if (si.purged) + continue; + + if (frameCount - si.lastUsedFrameCount > SUBVIEW_IMAGE_KILL_AFTER_FRAMES) { + si.purged = true; + si.image->PurgeImage(); + } + } +} + +idImageScratch *idRenderSystemLocal::CreateImageForSubview() { + // should be called from frontend + assert(!com_smp.GetBool() || session->IsFrontend()); + + renderCrop_t &rc = renderCrops[currentRenderCrop]; + int width = rc.width; + int height = rc.height; + + int slot = -1; + for (int i = 0; i < subviewImages.Num(); i++) { + ImageForSubview &si = subviewImages[i]; + + if (si.purged) { + slot = i; + continue; // GPU texture was freed + } + + if (si.lastUsedFrameCount == frameCount) { + continue; // already occupied for this frame + } + + if (si.width == width && si.height == height) { + // can reuse texture (not freed yet) + si.lastUsedFrameCount = frameCount; + return si.image; + } + } + + if (slot < 0) { + // add new texture to the end + ImageForSubview &added = subviewImages.Alloc(); + slot = subviewImages.IndexOf(&added); + added.width = width; + added.height = height; + + idStr imageName = va("$subview$%d", slot); + added.image = globalImages->ImageScratch(imageName); + } + + ImageForSubview &si = subviewImages[slot]; + si.purged = false; // will be generated in backend when data is copied into it + si.lastUsedFrameCount = frameCount; + return si.image; +} + static int RoundDownToPowerOfTwo( int v ) { int i; Index: renderer/tr_local.h =================================================================== --- renderer/tr_local.h (revision 10623) +++ renderer/tr_local.h (working copy) @@ -153,6 +153,7 @@ const struct viewEntity_s *space; const idMaterial *material; // may be NULL for shadow volumes + idImage *dynamicImageOverride; // stgatilov: if not NULL, then texture of material should be replaced with this one (for subviews) float sort; // material->sort, modified by gui / entity sort offsets const float *shaderRegisters; // evaluated and adjusted for referenceShaders /*const*/ struct drawSurf_s *nextOnLight; // viewLight chains @@ -819,6 +820,13 @@ static const int MAX_RENDER_CROPS = 8; +struct ImageForSubview { + bool purged = false; + int width = -1, height = -1; + idImageScratch *image = nullptr; + int lastUsedFrameCount = INT_MIN; +}; + /* ** Most renderer globals are defined here. ** backend functions should never modify any of these fields, @@ -877,6 +885,9 @@ void Clear( void ); void RenderViewToViewport( const renderView_t &renderView, idScreenRect &viewport ); + idImageScratch * CreateImageForSubview(); + void FreeOldSubviewImages(); + public: // renderer globals bool takingScreenshot; @@ -931,6 +942,8 @@ unsigned short gammaTable[256]; // brightness / gamma modify this idParallelJobList* frontEndJobList; + + idList<ImageForSubview> subviewImages; }; extern backEndState_t backEnd; |
|
Thief looking black in the mirror is not a bug. Thief looks normal in AC2 and on another test map. However, I noticed that no light sources interact with player model: it is only lit by ambient light. The test map used here had no ambient light. As soon as I added it, the thief was no longer black. |
|
Speaking of the sky. First, there are two techniques of settings up where sky is. One is to fill spaces with portalsky, another one is to leave caulk so that skybox implicitly remains in framebuffer and is not overdrawn with anything else. I'm not sure yet whether this choice affects what happens here. Another variation is the "type" spawnarg on "info_portalsky" entity: 0 "default": viewpoint is always at the origin of portalsky, thus skybox does not move relative to viewer 1 "global": viewpoint moves along with player with some "scale" multiplier, so items in skybox scene move relative to the viewer 2 "local" same as global, but I think it does not move when it is not active All three types are present in map/skiestdm.map. There are two missions which use 1 "global": Williame Steele 2 and 3. All the other usages stick to 0 "default". The default mode only allows to render infinitely far objects. Non-default modes allow to render objects at certain distance which are certainly behind normal scene. I suppose it is useful when you want to make some background geometry too large for a normal level. The local mode seems useless to me because it you exit sky zone at point A then reenter it at point B and go back to point A, the sky will look different? I'd say mapper needs functional dependence of sky viewport on player viewport. So, this variation of sky makes it a difficult to support sky in subviews. I think I'll concentrate on default skybox and portalsky, and include other cases only if it's easy. |
|
Lots of changes committed: r10701 Fixed skybox in mirrors. r10702 It is now possible to have recursively nested subviews (although inefficient). The story of skyboxes ended pretty easily. The default mode sets camera origin to fixed point, so it works equally fine if skybox is nested into some other subview. There was minor issue with mirroring: portal subview did not inherit properties of the parent subview, now they do. Several clip planes for nested mirrors are not necessary. The most-nested clip plane cuts all view rays later than any parent clip plane, so only most-nested clip plane is set now. X-ray also support now. The most annoying feature of X-ray is that it can be generated from GUI overlay. This code is refactored now a bit. The ugliest thing is using subview output in custom shaders (e.g. heatHaze). Unlike simple texture stages, custom shader stages accept many textures as inputs, and it is not clear which of them should be overriden =( |
|
Latest SVN build seems to have broken _scratch image behavior and portalsky behavior in the grass demo map. | |
Where can I get this grass demo? | |
https://www.moddb.com/mods/the-dark-mod/addons/arcturuss-animated-grass-demo | |
Fixed usage of mirrorRenderMap (or other subview generators) with custom shaders: r10711 Fixed custom shaders which use dynamic textures (not only X-ray case). The demo is not completely broken, but I'm not sure it works as intended. There are two surfaces with "arcturus_pool" material. One successfully uses the mirrored image, the other one does not (and uses black image instead). The reason why that surface does NOT generate a subview is that some of its triangles are backfacing. The engine only supports planar mirror surfaces: non-planar ones result in pretty undefined behavior. |
|
In general, I think at some moment we'll have to do some cleanup of materials with dynamic textures. Here is the list of dynamic textures we have now. ::: _cinematic Filled via UploadScratch from video file at the moment of rendering. Only used internally, no external references are allowed. ::: _guiRender, _shadowDepthFbo, _shadowAtlas, _currentStencilFbo Only filled and used internally as framebuffer attachments, no material may reference them. ::: _currentRender, _currentDepth Filled internally as framebuffer attachments, but exposed to postprocessing materials. Materials often reference them in order to implement effects like heatHaze, and thus they are public. Their meaning/behavior is fixed and should not change over time. ::: _scratch This image has mixed purpose. On one hand, it is sometimes used in hardcoded ways (screen wipe, doublevision, etc.). In these cases it is filled from C++ code in special way, and may only be used by a few core materials --- i.e. is completely private. On the other hand, it is often used as destination of subview-generated images, including user-defined materials (see below). In this case it becomes rather public and is referenced by materials ::: _dynamic, _scratch2, _xray, _cameraN (and partially _scratch) These images are subview-generated and are referenced in materials, including user ones. After this ticket, all these textures never actually exist and are never filled with data, they are merely placeholders in material text to mark that subview-generated image should be used instead. 1) If you have a custom shader stage (the one like heatHaze) with dynamic texture generation (e.g. mirrorRenderMap), then each reference to any of these texture is replaced with the texture generated by the subview. 2) If you have "simple texture" / "old" stage with dynamic texture generation, then subview-generated texture is used REGARDLESS of which texture you specify. This is because e.g. "textures/common/mirror" material references non-existent "textures/common/mirror" texture. Perhaps we should fix these materials and make rules the same. |
|
Thanks for the water fix! The sky is still rendering black and this is happening for other missions as well ( For example: "Mission 1: A New Job" , "Written in Stone" ) I assume portalsky behavior is still WIP ? |
|
Fixed here: r10712 Restored portalsky subview generation. In fact, I made sure skybox works fine even in nested mirrors, but accidentally deleted portalsky subview generation later when I worked on X-ray. |
|
Thank you! Everything is functional again! I have mixed feelings about the whole "scratch" concept. The fact that it is de-facto volatile makes me wish that we had multiple _currentRender images (etc) so we can define where we capture the image in comparison to scene completion. I guess "sort flags" hand that, but it might be better if it were implicit in the image? |
|
Date Modified | Username | Field | Change |
---|---|---|---|
06.01.2024 16:31 | stgatilov | New Issue | |
06.01.2024 16:31 | stgatilov | Status | new => assigned |
06.01.2024 16:31 | stgatilov | Assigned To | => stgatilov |
06.01.2024 16:31 | stgatilov | Relationship added | related to 0006424 |
06.01.2024 16:36 | stgatilov | Note Added: 0016326 | |
08.01.2024 14:54 | stgatilov | Note Added: 0016347 | |
08.01.2024 15:05 | stgatilov | Note Added: 0016348 | |
08.01.2024 15:05 | stgatilov | File Added: 6434_nested_subviews_wip.patch | |
08.01.2024 15:07 | stgatilov | Note Edited: 0016348 | |
08.01.2024 15:13 | stgatilov | Note Edited: 0016348 | |
04.04.2024 20:37 | stgatilov | Note Added: 0016617 | |
06.04.2024 08:50 | stgatilov | Relationship added | related to 0003108 |
06.04.2024 09:19 | stgatilov | Note Added: 0016619 | |
11.04.2024 19:52 | stgatilov | Note Added: 0016635 | |
11.04.2024 19:52 | stgatilov | Status | assigned => feedback |
13.04.2024 03:52 | nbohr1more | Note Added: 0016640 | |
13.04.2024 03:52 | nbohr1more | Note Added: 0016641 | |
13.04.2024 03:52 | nbohr1more | File Added: grass (2024-04-12 23-51-07) (240.73 -819.45 -337.12).jpg | |
13.04.2024 08:17 | stgatilov | Note Added: 0016642 | |
13.04.2024 12:32 | nbohr1more | Note Added: 0016643 | |
13.04.2024 14:07 | stgatilov | Note Added: 0016646 | |
13.04.2024 14:26 | stgatilov | Note Added: 0016647 | |
13.04.2024 14:56 | nbohr1more | Note Added: 0016648 | |
13.04.2024 16:42 | stgatilov | Note Added: 0016649 | |
13.04.2024 18:42 | nbohr1more | Note Added: 0016650 | |
14.04.2024 02:07 | nbohr1more | Note Edited: 0016650 |