Page MenuHomeWildfire Games
Differential D4493

Moves vertex and index buffer management to CDeviceCommandContext
ClosedPublic
Actions

Authored by vladislavbelov on Feb 16 2022, 7:54 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP26406: Moves GL vertex and index buffer management to CDeviceCommandContext.
Summary

First step to abstract vertex attributes manipulations. Also we need to rename/refactor VertexArray/VertexBuffer as they're used not only for vertices but also for indices.

Test Plan
  1. Apply the patch and compile the game
  2. Make sure the game works in Release and Debug modes for GL and GL ARB
  3. Compare performance for some complex scene
  4. Check Atlas and terrain manipulations (like painting and sculpting)

Diff Detail

Event Timeline

vladislavbelov created this revision.Feb 16 2022, 7:54 PM
Vulcan added a comment.Feb 16 2022, 8:06 PM

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

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

Vulcan added a comment.Feb 16 2022, 8:17 PM

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

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

Vulcan added a comment.Feb 16 2022, 8:34 PM

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

builderr-release-gcc7.txt
In file included from ../../../source/pch/atlas/precompiled.h:26:
../../../source/tools/atlas/GameInterface/Messages.h: In function 'void AtlasMessage::fGetTerrainGroupPreviews(AtlasMessage::qGetTerrainGroupPreviews*)':
../../../source/tools/atlas/GameInterface/Messages.h:305:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
 struct sTerrainTexturePreview
        ^~~~~~~~~~~~~~~~~~~~~~
../../../source/tools/atlas/GameInterface/Messages.h:305:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
../../../source/tools/atlas/GameInterface/Messages.h:305:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
../../../source/tools/atlas/GameInterface/Messages.h:305:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
 struct sTerrainTexturePreview
        ^~~~~~~~~~~~~~~~~~~~~~
../../../source/tools/atlas/GameInterface/Messages.h:305:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
../../../source/tools/atlas/GameInterface/Messages.h:305:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]

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

Harbormaster completed remote builds in B19511: Diff 19691.Feb 16 2022, 8:34 PM
vladislavbelov requested review of this revision.Feb 16 2022, 8:34 PM
Stan added a subscriber: Stan.Feb 17 2022, 12:47 AM
Stan added inline comments.
source/renderer/VertexBuffer.h
161

memory

source/renderer/backend/gl/DeviceCommandContext.cpp
116

Nullptr?

130

Should it be logmessage?

vladislavbelov marked 3 inline comments as done.Feb 17 2022, 8:44 AM
vladislavbelov added inline comments.
source/renderer/backend/gl/DeviceCommandContext.cpp
130

No, it might happen during window resizing for some platforms. I don't think we need warning on each resize.

Stan added inline comments.Feb 17 2022, 8:47 AM
source/renderer/backend/gl/DeviceCommandContext.cpp
130

Logmessage only shows up in the mainlog.
You wouldn't see it :)
There is logmessagerender for that purpose iirc.

vladislavbelov marked 2 inline comments as done.Feb 17 2022, 12:33 PM
vladislavbelov added inline comments.
source/renderer/backend/gl/DeviceCommandContext.cpp
130

Ah, ok, that makes sense.

vladislavbelov updated this revision to Diff 19695.Feb 17 2022, 12:41 PM
vladislavbelov marked an inline comment as done.
Vulcan added a comment.Feb 17 2022, 12:52 PM

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

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

Vulcan added a comment.Feb 17 2022, 12:57 PM

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

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

Vulcan added a comment.Feb 17 2022, 1:13 PM

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

builderr-release-gcc7.txt
In file included from ../../../source/pch/atlas/precompiled.h:26:
../../../source/tools/atlas/GameInterface/Messages.h: In function 'void AtlasMessage::fGetTerrainGroupPreviews(AtlasMessage::qGetTerrainGroupPreviews*)':
../../../source/tools/atlas/GameInterface/Messages.h:305:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
 struct sTerrainTexturePreview
        ^~~~~~~~~~~~~~~~~~~~~~
../../../source/tools/atlas/GameInterface/Messages.h:305:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
../../../source/tools/atlas/GameInterface/Messages.h:305:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
../../../source/tools/atlas/GameInterface/Messages.h:305:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
 struct sTerrainTexturePreview
        ^~~~~~~~~~~~~~~~~~~~~~
../../../source/tools/atlas/GameInterface/Messages.h:305:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
../../../source/tools/atlas/GameInterface/Messages.h:305:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]

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

Harbormaster completed remote builds in B19515: Diff 19695.Feb 17 2022, 1:13 PM
vladislavbelov updated this revision to Diff 19696.Feb 17 2022, 4:48 PM
Vulcan added a comment.Feb 17 2022, 5:00 PM

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

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

Vulcan added a comment.Feb 17 2022, 5:04 PM

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

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

Vulcan added a comment.Feb 17 2022, 5:20 PM

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

builderr-release-gcc7.txt
In file included from ../../../source/pch/atlas/precompiled.h:26:
../../../source/tools/atlas/GameInterface/Messages.h: In function 'void AtlasMessage::fGetTerrainGroupPreviews(AtlasMessage::qGetTerrainGroupPreviews*)':
../../../source/tools/atlas/GameInterface/Messages.h:305:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
 struct sTerrainTexturePreview
        ^~~~~~~~~~~~~~~~~~~~~~
../../../source/tools/atlas/GameInterface/Messages.h:305:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
../../../source/tools/atlas/GameInterface/Messages.h:305:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
../../../source/tools/atlas/GameInterface/Messages.h:305:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
 struct sTerrainTexturePreview
        ^~~~~~~~~~~~~~~~~~~~~~
../../../source/tools/atlas/GameInterface/Messages.h:305:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]
../../../source/tools/atlas/GameInterface/Messages.h:305:8: warning: '#'target_mem_ref' not supported by dump_expr#<expression error>' may be used uninitialized in this function [-Wmaybe-uninitialized]

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

Harbormaster completed remote builds in B19516: Diff 19696.Feb 17 2022, 5:20 PM
Stan added a comment.Feb 18 2022, 5:16 PM

Seems to work for me.

This revision was not accepted when it landed; it landed in state Needs Review.Feb 18 2022, 6:33 PM
This revision was landed with ongoing or failed builds.
Closed by commit rP26406: Moves GL vertex and index buffer management to CDeviceCommandContext. (authored by vladislavbelov). · Explain Why
This revision was automatically updated to reflect the committed changes.
vladislavbelov added a commit: rP26406: Moves GL vertex and index buffer management to CDeviceCommandContext..

Revision Contents
Changeset List

Diff 19695

View Options

source/graphics/MiniMapTexture.cpp

Loading...
View Options

source/graphics/ParticleEmitter.h

Loading...
View Options

source/graphics/ParticleEmitter.cpp

Loading...
View Options

source/renderer/DecalRData.cpp

Loading...
View Options

source/renderer/HWLightingModelRenderer.h

Loading...
View Options

source/renderer/HWLightingModelRenderer.cpp

Loading...
View Options

source/renderer/InstancingModelRenderer.h

Loading...
View Options

source/renderer/InstancingModelRenderer.cpp

Loading...
View Options

source/renderer/ModelRenderer.cpp

Loading...
View Options

source/renderer/ModelVertexRenderer.h

Loading...
View Options

source/renderer/OverlayRenderer.cpp

Loading...
View Options

source/renderer/ParticleRenderer.cpp

Loading...
View Options

source/renderer/PatchRData.h

Loading...
View Options

source/renderer/PatchRData.cpp

Loading...
View Options

source/renderer/TerrainRenderer.cpp

Loading...
View Options

source/renderer/TexturedLineRData.cpp

Loading...
View Options

source/renderer/VertexArray.h

Loading...
View Options

source/renderer/VertexArray.cpp

Loading...
View Options

source/renderer/VertexBuffer.h

Loading...
View Options

source/renderer/VertexBuffer.cpp

Loading...
View Options

source/renderer/VertexBufferManager.h

Loading...
View Options

source/renderer/VertexBufferManager.cpp

Loading...
View Options

source/renderer/WaterManager.cpp

Loading...
View Options

source/renderer/backend/gl/Buffer.h

Loading...
View Options

source/renderer/backend/gl/Buffer.cpp

Loading...
View Options

source/renderer/backend/gl/Device.h

Loading...
View Options

source/renderer/backend/gl/Device.cpp

Loading...
View Options

source/renderer/backend/gl/DeviceCommandContext.h

Loading...
View Options

source/renderer/backend/gl/DeviceCommandContext.cpp

Loading...
Wildfire Games · Phabricator