View Issue Details

IDProjectCategoryView StatusLast Update
0004417The Dark ModCodingpublic22.11.2016 19:51
Reporterduzenko Assigned Toduzenko  
PrioritynormalSeveritynormalReproducibilityhave not tried
Status closedResolutionwon't fix 
Product VersionSVN 
Target VersionSVN 
Summary0004417: See if surfaces need to write to color buffer at the depth stage
DescriptionCurrently the depth stage writes black to the color buffer to initialize pixels for additive blending.
Theoretically since AFAIK unlike Doom3 all surfaces are lit by the ambient light it might make sense to use the ambient light as initialization and make the color buffer read-only at the depth stage.
It seems that currently light interactions are rendered before ambient so the first step would be change the order to do ambient first.
TagsNo tags attached.

Relationships

related to 0004414 resolvedduzenko See if skybox can be rendered faster 

Activities

duzenko

duzenko

15.11.2016 21:21

developer   ~0008487

Which glproc is used for ambient now?
We should change it (create new?) from "read-add-write" to "write".
Or, if it's done via glBlendFunc, change dfactor from gl_one to gl_zero
nbohr1more

nbohr1more

15.11.2016 21:28

developer   ~0008488

Last edited: 15.11.2016 21:32

View 2 revisions

If the Enhanced Ambient is used, cv_ambient_method = 0
the ambientLight.vfp glprog is active.

If the Simple ambient is used, cv_ambient_method = 1
there is an additive brightening stage (blend add):

// The following two blocks are required for the ambient shading:
        // TDM Ambient Method Related
        {
                if (global5 == 1)
                blend add
                map textures/darkmod/your_file_folder/your_texture_name
                scale 1, 1
                red global2
                green global3
                blue global4
        }

in the material definitions.

Interesting. If you make this light-weight enough, then we could probably just drop the simple ambient performance option.

duzenko

duzenko

16.11.2016 14:14

developer   ~0008490

Can't see blending code in ambientLight.vfp.
nbohr1more

nbohr1more

16.11.2016 14:20

developer   ~0008491

draw_arb2.cpp is where the interaction shader blend happens.

I believe that draw_common.cpp handles the simple material gl blend operations.

Also, I should mention that the glprog is only for typical ambient lights. Mappers can choose regular lights for their "ambient_world" light. That would mean
any glprog at the bottom of draw_arb2.cpp could act as the ambient_world light.


Look at ambient_world references in (game) Player_View.cpp and game_local.cpp to see how that's handled on the game side.
duzenko

duzenko

16.11.2016 16:13

developer   ~0008492

Last edited: 17.11.2016 15:57

View 5 revisions

Skip color buffer in depth stage - done in RB_STD_FillDepthBuffer

    if (r_ignore.GetBool())
        GL_State(GLS_DEPTHFUNC_LESS | GLS_COLORMASK);
    else
        GL_State(GLS_DEPTHFUNC_LESS);

Draw ambient surfaces first - done in RB_STD_DrawView.

    RB_STD_FillDepthBuffer( drawSurfs, numDrawSurfs );

    if (r_ignore.GetBool())
        processed = RB_STD_DrawShaderPasses(drawSurfs, numDrawSurfs);

Now ambient surfaces are transparent and I can't see how to disable dest blending for them.
...
The problem is the skybox blending with everything.

nbohr1more

nbohr1more

16.11.2016 16:32

developer   ~0008493

You probably need to use another buffer or MRT. The engine draws additive so you are starting with the skybox and adding the other surfaces to the same buffer over the top of them.

(The material def for the skybox itself typically a blend add operation so that may be part of the problem here. Though, if it were changed we may be back to having problems with crack decals not rendering over it (etc)).
duzenko

duzenko

16.11.2016 16:34

developer   ~0008494

Yes, most materials have an explicit blend add flag. Overriding that fixes the skybox but breaks transparent and self-lit materials.
nbohr1more

nbohr1more

16.11.2016 16:39

developer   ~0008495

One option is to create a new blend alias "blend sky" then update all the material definitions for sky boxes with it.
duzenko

duzenko

16.11.2016 16:40

developer   ~0008496

Last edited: 16.11.2016 16:42

View 2 revisions

It appears to be the other way around - skybox is ok, it's world materials that blend with it.
...
Your idea might work if we rendered skybox after the world.

nbohr1more

nbohr1more

16.11.2016 16:52

developer   ~0008497

Well, that idea was not ideal anyway. Any custom sky assets packed with missions would also need to be updated.

Not sure how you would alter it but this is what you are fighting against in draw_arb2.cpp

GL_State( GLS_SRCBLEND_ONE | GLS_DSTBLEND_ONE | GLS_DEPTHMASK | backEnd.depthFunc );
duzenko

duzenko

16.11.2016 16:54

developer   ~0008498

Last edited: 16.11.2016 16:57

View 3 revisions

No, that's RB_ARB2_CreateDrawInteractions and light interactions are fine.
idMaterial::ParseBlend on the other hand makes a lot of difference.
...
That stage->drawStateBits flag is used in RB_STD_T_RenderShaderPasses
...
What I can't understand is how overriding drawStateBits from additive to default suddenly breaks smoke.

nbohr1more

nbohr1more

16.11.2016 17:03

developer   ~0008499

Is smoke broken if you set:

r_useSoftParticles 0 ?
duzenko

duzenko

16.11.2016 18:12

developer   ~0008500

Can't check right now but probably yes, because even console is screwed up.
duzenko

duzenko

17.11.2016 11:15

developer   ~0008503

Actually, r_useSoftParticles 0 is broken (black tris) but r_useSoftParticles 1 is ok
duzenko

duzenko

17.11.2016 15:57

developer   ~0008504

Last edited: 17.11.2016 16:00

View 2 revisions

Most surface are ok with standard ambient. Windows are not lit, and some HUD black.
if (r_ignore.GetBool() && ((pStage->drawStateBits & 0xff) == (GLS_SRCBLEND_ONE | GLS_DSTBLEND_ONE)))
            GL_State(pStage->drawStateBits & 0xffffff00);
else
            GL_State(pStage->drawStateBits);

duzenko

duzenko

17.11.2016 20:51

developer   ~0008505

Well, I got it working but fps gain is so marginal that it's probably not even worth committing.
nbohr1more

nbohr1more

17.11.2016 21:14

developer   ~0008506

Thanks for all your efforts. Even if it didn't yield much improvement, it will inform any future developers about what has already been tried.

In the interest of risk reduction for unforeseen issues, I agree that this is probably best left out (unless you knew it would help for future development changes).

I will now close this case.
duzenko

duzenko

17.11.2016 21:23

developer  

1.diff (2,148 bytes)   
Index: draw_common.cpp
===================================================================
--- draw_common.cpp	(revision 6675)
+++ draw_common.cpp	(working copy)
@@ -574,7 +574,10 @@
 	// decal surfaces may enable polygon offset
 	qglPolygonOffset( r_offsetFactor.GetFloat(), r_offsetUnits.GetFloat() );
 
-	GL_State( GLS_DEPTHFUNC_LESS );
+	if (r_ignore.GetBool())
+		GL_State(GLS_DEPTHFUNC_LESS | GLS_COLORMASK);
+	else
+		GL_State(GLS_DEPTHFUNC_LESS);
 
 	// Enable stencil test if we are going to be using it for shadows.
 	// If we didn't do this, it would be legal behavior to get z fighting
@@ -831,6 +834,7 @@
 	idDrawVert *ac = (idDrawVert *)vertexCache.Position( tri->ambientCache );
 	qglVertexPointer( 3, GL_FLOAT, sizeof( idDrawVert ), ac->xyz.ToFloatPtr() );
 	qglTexCoordPointer( 2, GL_FLOAT, sizeof( idDrawVert ), reinterpret_cast<void *>(&ac->st) );
+	bool additiveBlend = !r_ignore.GetBool();
 
 	for ( stage = 0; stage < shader->GetNumStages() ; stage++ ) {		
 		pStage = shader->GetStage(stage);
@@ -1109,8 +1113,16 @@
 		RB_BindVariableStageImage( &pStage->texture, regs );
 
 		// set the state
-		GL_State( pStage->drawStateBits );
-		
+		if (!additiveBlend && (
+			(pStage->drawStateBits & 0xff) == (GLS_SRCBLEND_ONE | GLS_DSTBLEND_ONE) ||
+			(pStage->drawStateBits & 0xff) == (GLS_SRCBLEND_ONE | GLS_DSTBLEND_ONE_MINUS_SRC_ALPHA)
+		)) {
+			GL_State(pStage->drawStateBits & 0xffffff00);
+			additiveBlend = true;
+		}
+		else
+			GL_State(pStage->drawStateBits);
+
 		RB_PrepareStageTexturing( pStage, surf, ac );
 
 		// draw it
@@ -1972,6 +1984,9 @@
 	// subviews
 	RB_STD_FillDepthBuffer( drawSurfs, numDrawSurfs );
 
+	// now draw any non-light dependent shading passes
+	processed = RB_STD_DrawShaderPasses(drawSurfs, numDrawSurfs);
+
 	// main light renderer
 	switch( tr.backEndRenderer ) {
 	case BE_ARB:
@@ -1997,9 +2012,6 @@
 	// uplight the entire screen to crutch up not having better blending range
 	RB_STD_LightScale();
 
-	// now draw any non-light dependent shading passes
-	processed = RB_STD_DrawShaderPasses( drawSurfs, numDrawSurfs );
-
 	// fob and blend lights
 	RB_STD_FogAllLights();
 
1.diff (2,148 bytes)   
duzenko

duzenko

17.11.2016 21:24

developer   ~0008507

Attached diff in case of future review

Issue History

Date Modified Username Field Change
15.11.2016 21:18 duzenko New Issue
15.11.2016 21:18 duzenko Status new => assigned
15.11.2016 21:18 duzenko Assigned To => duzenko
15.11.2016 21:21 duzenko Note Added: 0008487
15.11.2016 21:28 nbohr1more Note Added: 0008488
15.11.2016 21:32 nbohr1more Note Edited: 0008488 View Revisions
16.11.2016 14:14 duzenko Note Added: 0008490
16.11.2016 14:20 nbohr1more Note Added: 0008491
16.11.2016 16:13 duzenko Note Added: 0008492
16.11.2016 16:15 duzenko Note Edited: 0008492 View Revisions
16.11.2016 16:32 nbohr1more Note Added: 0008493
16.11.2016 16:34 duzenko Note Added: 0008494
16.11.2016 16:39 nbohr1more Note Added: 0008495
16.11.2016 16:40 duzenko Note Added: 0008496
16.11.2016 16:42 duzenko Note Edited: 0008496 View Revisions
16.11.2016 16:46 duzenko Note Edited: 0008492 View Revisions
16.11.2016 16:47 duzenko Note Edited: 0008492 View Revisions
16.11.2016 16:52 nbohr1more Note Added: 0008497
16.11.2016 16:54 duzenko Note Added: 0008498
16.11.2016 16:55 duzenko Note Edited: 0008498 View Revisions
16.11.2016 16:57 duzenko Note Edited: 0008498 View Revisions
16.11.2016 17:03 nbohr1more Note Added: 0008499
16.11.2016 18:12 duzenko Note Added: 0008500
17.11.2016 11:15 duzenko Note Added: 0008503
17.11.2016 15:57 duzenko Note Added: 0008504
17.11.2016 15:57 duzenko Note Edited: 0008492 View Revisions
17.11.2016 16:00 duzenko Note Edited: 0008504 View Revisions
17.11.2016 20:51 duzenko Note Added: 0008505
17.11.2016 21:14 nbohr1more Note Added: 0008506
17.11.2016 21:14 nbohr1more Status assigned => closed
17.11.2016 21:14 nbohr1more Resolution open => won't fix
17.11.2016 21:14 nbohr1more Product Version => SVN
17.11.2016 21:14 nbohr1more Target Version => SVN
17.11.2016 21:21 duzenko File Added: 1.txt
17.11.2016 21:22 duzenko File Deleted: 1.txt
17.11.2016 21:23 duzenko File Added: 1.diff
17.11.2016 21:24 duzenko Note Added: 0008507
22.11.2016 19:51 nbohr1more Relationship added related to 0004414