Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix union order to simplify empty initializers #101560

Merged
merged 1 commit into from
Jan 16, 2025
Merged

Fix union order to simplify empty initializers #101560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/templates/a_hash_map.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@

struct HashMapData {
union {
uint64_t data;
struct
{
uint32_t hash;
uint32_t hash_to_key;
};
uint64_t data;
};
};

Expand Down
8 changes: 4 additions & 4 deletions drivers/gles3/rasterizer_scene_gles3.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,10 @@ class RasterizerSceneGLES3 : public RendererSceneRender {
};

union {
struct {
uint64_t sort_key1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this initialize both keys, or just sort_key_1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand it will initialize both keys:

union initialization where only the very first element of the union is picked when using empty initializers such as {} if no other element in the union has an explicit initializer.

In this case the nameless struct is the first element in the union.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh okay. I misunderstood what was causing the problem. I thought the first element of the struct was being grabbed, but no. The problem in these cases is that the first struct was smaller than the second.

So a simple rule of thumb (that this PR enforces) is to always put the larger element first in a union

uint64_t sort_key2;
};
struct {
uint64_t lod_index : 8;
uint64_t surface_index : 8;
Expand All @@ -265,10 +269,6 @@ class RasterizerSceneGLES3 : public RendererSceneRender {
uint64_t depth_layer : 4;
uint64_t priority : 8;
};
struct {
uint64_t sort_key1;
uint64_t sort_key2;
};
} sort;

RS::PrimitiveType primitive = RS::PRIMITIVE_MAX;
Expand Down
2 changes: 1 addition & 1 deletion scene/3d/voxelizer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,13 @@ class Voxelizer {

struct CellSort {
union {
uint64_t key = 0;
struct {
uint64_t z : 16;
uint64_t y : 16;
uint64_t x : 16;
uint64_t level : 16;
};
uint64_t key = 0;
};

int32_t index = 0;
Expand Down
2 changes: 1 addition & 1 deletion scene/gui/graph_edit.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,11 +150,11 @@ class GraphEdit : public Control {
private:
struct ConnectionType {
union {
uint64_t key = 0;
struct {
uint32_t type_a;
uint32_t type_b;
};
uint64_t key = 0;
};

static uint32_t hash(const ConnectionType &p_conn) {
Expand Down
12 changes: 6 additions & 6 deletions servers/rendering/renderer_rd/forward_clustered/render_forward_clustered.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,10 @@ class RenderForwardClustered : public RendererSceneRenderRD {
};

union {
struct {
uint64_t sort_key1;
uint64_t sort_key2;
};
struct {
uint64_t lod_index : 8;
uint64_t surface_index : 8;
Expand All @@ -443,10 +447,6 @@ class RenderForwardClustered : public RendererSceneRenderRD {
uint64_t depth_layer : 4;
uint64_t priority : 8;
};
struct {
uint64_t sort_key1;
uint64_t sort_key2;
};
} sort;

RS::PrimitiveType primitive = RS::PRIMITIVE_MAX;
Expand Down Expand Up @@ -541,6 +541,8 @@ class RenderForwardClustered : public RendererSceneRenderRD {

struct GlobalPipelineData {
union {
uint32_t key;

struct {
uint32_t texture_samples : 3;
uint32_t use_reflection_probes : 1;
Expand All @@ -556,8 +558,6 @@ class RenderForwardClustered : public RendererSceneRenderRD {
uint32_t use_shadow_cubemaps : 1;
uint32_t use_shadow_dual_paraboloid : 1;
};

uint32_t key;
};
};

Expand Down
12 changes: 6 additions & 6 deletions servers/rendering/renderer_rd/forward_mobile/render_forward_mobile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,10 @@ class RenderForwardMobile : public RendererSceneRenderRD {
};

union {
struct {
uint64_t sort_key1;
uint64_t sort_key2;
};
struct {
// !BAS! CHECK BITS!!!

Expand All @@ -413,10 +417,6 @@ class RenderForwardMobile : public RendererSceneRenderRD {
// uint64_t lod_index : 8; // no need to sort on LOD
// uint64_t uses_forward_gi : 1; // no GI here, remove
};
struct {
uint64_t sort_key1;
uint64_t sort_key2;
};
} sort;

RS::PrimitiveType primitive = RS::PRIMITIVE_MAX;
Expand Down Expand Up @@ -575,6 +575,8 @@ class RenderForwardMobile : public RendererSceneRenderRD {

struct GlobalPipelineData {
union {
uint32_t key;

struct {
uint32_t texture_samples : 3;
uint32_t target_samples : 3;
Expand All @@ -586,8 +588,6 @@ class RenderForwardMobile : public RendererSceneRenderRD {
uint32_t use_shadow_cubemaps : 1;
uint32_t use_shadow_dual_paraboloid : 1;
};

uint32_t key;
};
};

Expand Down