Moves model flags to ModelAbstract
ClosedPublic
Actions

Authored by vladislavbelov on Oct 3 2023, 7:16 PM.

Details

Reviewers: None
Commits: rP27880: Moves model flags to ModelAbstract.

Summary

Another step to unify models and make materials be immutable. That patch isn't the cleanest but I think it's enough for now to not spend time on it.

Test Plan

Apply the patch and compile the game
Run tests

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vladislavbelov created this revision.Oct 3 2023, 7:16 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/7358/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

builderr-debug-gcc7.txt
../../../source/renderer/SceneRenderer.cpp: In member function 'void CSceneRenderer::RenderShadowMap(Renderer::Backend::IDeviceCommandContext*, const CShaderDefines&)':
../../../source/renderer/SceneRenderer.cpp:328:72: error: 'MODELFLAG_CASTSHADOWS' was not declared in this scope
    m->CallModelRenderers(deviceCommandContext, contextCast, cullGroup, MODELFLAG_CASTSHADOWS);
                                                                        ^~~~~~~~~~~~~~~~~~~~~
../../../source/renderer/SceneRenderer.cpp:333:78: error: 'MODELFLAG_CASTSHADOWS' was not declared in this scope
    m->CallTranspModelRenderers(deviceCommandContext, contextCast, cullGroup, MODELFLAG_CASTSHADOWS);
                                                                              ^~~~~~~~~~~~~~~~~~~~~
../../../source/renderer/SceneRenderer.cpp: In member function 'virtual void CSceneRenderer::SubmitNonRecursive(CModel*)':
../../../source/renderer/SceneRenderer.cpp:1031:27: error: 'MODELFLAG_SILHOUETTE_OCCLUDER' was not declared in this scope
   if (model->GetFlags() & MODELFLAG_SILHOUETTE_OCCLUDER)
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../source/renderer/SceneRenderer.cpp:1031:27: note: suggested alternative: 'CULL_SILHOUETTE_OCCLUDER'
   if (model->GetFlags() & MODELFLAG_SILHOUETTE_OCCLUDER)
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                           CULL_SILHOUETTE_OCCLUDER
../../../source/renderer/SceneRenderer.cpp:1033:27: error: 'MODELFLAG_SILHOUETTE_DISPLAY' was not declared in this scope
   if (model->GetFlags() & MODELFLAG_SILHOUETTE_DISPLAY)
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../source/renderer/SceneRenderer.cpp:1033:27: note: suggested alternative: 'str_MODE_SILHOUETTEDISPLAY'
   if (model->GetFlags() & MODELFLAG_SILHOUETTE_DISPLAY)
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
                           str_MODE_SILHOUETTEDISPLAY
../../../source/renderer/SceneRenderer.cpp:1039:29: error: 'MODELFLAG_CASTSHADOWS' was not declared in this scope
   if (!(model->GetFlags() & MODELFLAG_CASTSHADOWS))
                             ^~~~~~~~~~~~~~~~~~~~~
make[1]: *** [graphics.make:475: obj/graphics_Debug/SceneRenderer.o] Error 1
make: *** [Makefile:127: graphics] Error 2

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/9029/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Debug:
    14>e:\jenkins\workspace\vs2015-differential\source\renderer\scenerenderer.cpp(328): error C2065: 'MODELFLAG_CASTSHADOWS': undeclared identifier [E:\Jenkins\workspace\vs2015-differential\build\workspaces\vs2017\graphics.vcxproj]
    14>e:\jenkins\workspace\vs2015-differential\source\renderer\scenerenderer.cpp(333): error C2065: 'MODELFLAG_CASTSHADOWS': undeclared identifier [E:\Jenkins\workspace\vs2015-differential\build\workspaces\vs2017\graphics.vcxproj]
    14>e:\jenkins\workspace\vs2015-differential\source\renderer\scenerenderer.cpp(1031): error C2065: 'MODELFLAG_SILHOUETTE_OCCLUDER': undeclared identifier [E:\Jenkins\workspace\vs2015-differential\build\workspaces\vs2017\graphics.vcxproj]
    14>e:\jenkins\workspace\vs2015-differential\source\renderer\scenerenderer.cpp(1033): error C2065: 'MODELFLAG_SILHOUETTE_DISPLAY': undeclared identifier [E:\Jenkins\workspace\vs2015-differential\build\workspaces\vs2017\graphics.vcxproj]
    14>e:\jenkins\workspace\vs2015-differential\source\renderer\scenerenderer.cpp(1039): error C2065: 'MODELFLAG_CASTSHADOWS': undeclared identifier [E:\Jenkins\workspace\vs2015-differential\build\workspaces\vs2017\graphics.vcxproj]

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/8447/display/redirect

Harbormaster completed remote builds in B22426: Diff 22356.Oct 3 2023, 7:23 PM

vladislavbelov requested review of this revision.Oct 3 2023, 7:23 PM

Using constants instead of macros is definitly good.
There is a mixture of uint32 and int.
Why not use std::bitset?

source/graphics/ModelAbstract.h
36 ↗	(On Diff #22356)	There is a tab. I would use `{}` for initialization. I don't mind that much.

vladislavbelov updated this revision to Diff 22358.Oct 3 2023, 11:08 PM

vladislavbelov marked an inline comment as done.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/7360/display/redirect

In D5146#219006, @phosit wrote:

Using constants instead of macros is definitly good.
There is a mixture of uint32 and int.

I plan to improve that in the next patches.

Why not use std::bitset?

Hmm, it provides more functionality than we need for flags. It might have some cost, but I haven't investigated it. Anyway it'd be too complex for planned changes, because the current goal is to improve materials, not models itself (at least not yet).

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/8449/display/redirect

Harbormaster completed remote builds in B22428: Diff 22358.Oct 3 2023, 11:33 PM

How about using an enum class ?

In D5146#219029, @Stan wrote:

How about using an enum class ?

My points about which I'm worrying: a) we can't use bit operations for enum class without explicit providing operators b) a | b doesn't belong to enum values.

Maybe that's a C# thing to have an enum like so:

[Flags]
public enum ModelFlags
{
CAST_SHADOWS		1,
NO_LOOP_ANIMATION	2,
SILHOUETTE_DISPLAY	4,
SILHOUETTE_OCCLUDER	8,
IGNORE_LOS		16,
FLOAT_ON_WATER		32,
}

var flags = ModelFlags.CAST_SHADOWS|ModelFlags.NO_LOOP_ANIMATION; //  3
// C# also has some nice stuff like converting to string easily and whatnot but we don't need that here.