One of preparations for compute shaders changes.
Details
- Reviewers
- None
- Commits
- rP27879: Merges UID from different Vulkan device objects and unifies single type…
- Apply the patch and compile the game
- Install Vulkan SDK (if not installed)
- Add the following lines to user.cfg:
renderer.backend.debugcontext = "true" renderer.backend.debuglabels = "true" renderer.backend.debugmessages = "true" renderer.backend.debugscopedlabels = "true"
- Run the game and resize a window
- Make sure that there is no new warnings
- Add the following line to user.cfg:
renderer.backend.vulkan.disabledescriptorindexing = "true"
- Run the game and resize a window
- Make sure that there is no new warnings
Do not forget to remove added lines as they might affect performance.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Lint Skipped - Unit
Unit Tests Skipped - Build Status
Buildable 22422 Build 54872: Vulcan Build Jenkins Build 54871: Vulcan Build (macOS) Jenkins Build 54870: Vulcan Build (Windows) Jenkins
Event Timeline
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/macos-differential/7337/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/8426/display/redirect
This diff weakens (syntactical) typesavety. (semanticly there is only uint32_t)
UID is to generic IMO. DeviceObjectUID might work.
I don't see where the upper bits are used as type, all UIDs are generated by GenerateNextUID which returns 1,2,3...
Why don't you use bitfields?
struct UID { u32 type : 2; u32 id : 30; };
source/renderer/backend/vulkan/Texture.cpp | ||
---|---|---|
337–341 | Why declare it at all. | |
341–345 | Removing this generates much duplication. |
Yeah, because the UID should enumerate all resources created by a device.
UID is to generic IMO. DeviceObjectUID might work.
I don't mind DeviceObjectUID.
I don't see where the upper bits are used as type, all UIDs are generated by GenerateNextUID which returns 1,2,3...
Why don't you use bitfields?
I was thinking about adding types/categories (I've added todo) to UID and I'd like to add them but I haven't came up with a satisfying solution yet (though I haven't spent much time on it).
source/renderer/backend/vulkan/Texture.cpp | ||
---|---|---|
341–345 | Yeah, I was thinking about that. I have no objections to pass CDevice as a parameter (though it'd be inconsistent with other device objects). |
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/macos-differential/7356/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/8445/display/redirect
It's better if DeviceObjectUID is a class/struct so it's not implicitly convertible from/to an integer.
source/renderer/backend/vulkan/DeviceObjectUID.h | ||
---|---|---|
36–39 | "It's used to describe the type" no it's not, it might be in future. | |
source/renderer/backend/vulkan/Texture.cpp | ||
338 | Can't be nullptr so it should be a reference. |
Only via assignments (which we should avoid anyway). Constructors work for both:
using UID = uint32_t; // or struct UID { uint32_t value; }; UID uid{42};
source/renderer/backend/vulkan/Texture.cpp | ||
---|---|---|
338 | Technically it can. But yeah, I want references in the future for backend objects. |