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.

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

Vulkan dump resources: Option to dump alpha separately #1883

Merged

Conversation

panos-lunarg
Copy link
Contributor

New option --dump-resources-dump-separate-alpha will force the alpha channel of dumped images to be dumped into a separate file

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 308731.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5376 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5376 failed.

@panos-lunarg panos-lunarg force-pushed the vulkan_dump_resources_split_alpha branch from 035e2c3 to c6dd4a6 Compare November 25, 2024 09:28
@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 308753.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5377 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5377 passed.

if (filename_before != nullptr)
{
json_entry["beforeFile"] = *filename_before;
json_entry["beforeFile-alpha"] = util::filepath::InsertFilenamePostfix(*filename_before, "_alpha");
Copy link
Contributor

@dfriederich dfriederich Nov 25, 2024

Choose a reason for hiding this comment

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

Could we change the -alpha attributes to follow the same naming rules as the other attributes,
say using "beforeFileAlpha", "afterFileAlpha", etc.

using a "-" does make attributes names slightly uglier to use in javascript, basically simple identifiers cannot have a - (I could handle it, but I think we should just use snakeCase here as well, just as we do for all the other attributes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

@panos-lunarg panos-lunarg force-pushed the vulkan_dump_resources_split_alpha branch from c6dd4a6 to 1b41390 Compare November 26, 2024 07:14
@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 309676.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5388 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5388 passed.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 310069.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5396 running.

@@ -96,6 +97,15 @@ static size_t temporary_buffer_size = 0;
} \
}

static void ResizeTemoraryBuffer(size_t new_size)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could the temporary be vector<uint8_t> and then you can use resize?

Copy link
Contributor

Choose a reason for hiding this comment

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

That is to say, you could call resize instead of calling this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

@@ -331,6 +334,73 @@ static const uint8_t* ConvertIntoTemporaryBuffer(uint32_t width,
return reinterpret_cast<const uint8_t*>(temporary_buffer.get());
}

static uint8_t* ExtractAlphaChannel(uint32_t width, uint32_t height, const void* data, uint32_t data_pitch, bool is_png)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be expand_to_rgb and invert the sense of the comparison? I'd like if possible for parameters to generally express the lowest meaningful thing; if we were to add another luminance-capable image then a future reader would need to guess is_png means "can have one-channel encoding".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5396 passed.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 310963.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5410 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5410 failed.

@panos-lunarg panos-lunarg force-pushed the vulkan_dump_resources_split_alpha branch from cfcb990 to 2ab8465 Compare November 27, 2024 15:18
@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 311383.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5419 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5419 passed.

@dfriederich
Copy link
Contributor

Works for me :-).
Thanks for updating the attribute names.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 312153.

panos-lunarg and others added 3 commits November 28, 2024 10:01
New option --dump-resources-dump-separate-alpha will force the alpha
channel of dumped images to be dumped into a separate file
@panos-lunarg panos-lunarg force-pushed the vulkan_dump_resources_split_alpha branch from 4f0e975 to b3f135e Compare November 28, 2024 08:02
@ci-tester-lunarg
Copy link

CI gfxreconstruct build queued with queue ID 312155.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5441 running.

@ci-tester-lunarg
Copy link

CI gfxreconstruct build # 5441 passed.

@panos-lunarg panos-lunarg merged commit 8a7aee6 into LunarG:dev Nov 28, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants