Skip to content

Add image dump in PNG#302

Merged
dj2 merged 8 commits intogoogle:masterfrom
dj2:png
Feb 20, 2019
Merged

Add image dump in PNG#302
dj2 merged 8 commits intogoogle:masterfrom
dj2:png

Conversation

@dj2
Copy link
Collaborator

@dj2 dj2 commented Feb 20, 2019

This change adds support to dump image to PNG format, it relies on the
third party 'lodepng' library (zlib license).

This CL is based on Pull #301 by @hevrard.

@dj2
Add image dump in PNG
a15fdad 
This change adds support to dump image to PNG format, it relies on the
third party 'lodepng' library (zlib license).

This CL is based on Pull #301 by @hevrard.
@dj2 dj2 added the enhancement New feature or request label Feb 20, 2019
@dj2 dj2 self-assigned this Feb 20, 2019
@dj2 dj2 requested a review from dneto0 February 20, 2019 16:58
@dj2
Copy link
Collaborator Author

dj2 commented Feb 20, 2019

@hevrard I think this should work ....

dneto0
dneto0 requested changes Feb 20, 2019
View reviewed changes
Copy link
Collaborator

@dneto0 dneto0 left a comment

Choose a reason for hiding this comment

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

Seems like a buffer overrun?
Less seriously: can do the vector-to-string conversion more efficiently, with less code.

samples/amber.cc Outdated
if (!options.image_filename.empty()) {
std::string image;
bool usePNG = options.image_filename.compare(
options.image_filename.length() - 5, options.image_filename.length(),
Copy link
Collaborator

@dneto0 dneto0 Feb 20, 2019

Choose a reason for hiding this comment

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

What if the string is very short, e.g. length is 2, then 2-5 is large negative that is then converted to unsigned.... out of bounds?

Copy link
Collaborator Author

@dj2 dj2 Feb 20, 2019

Choose a reason for hiding this comment

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

done

samples/png.cc

namespace {

unsigned char byte0(uint32_t word) {
Copy link
Collaborator

@dneto0 dneto0 Feb 20, 2019

Choose a reason for hiding this comment

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

Normally I'd ask for uint8_t here. But lodepng wants a vector of unsigned char. So I'll let this pass.

Copy link
Collaborator Author

@dj2 dj2 Feb 20, 2019

Choose a reason for hiding this comment

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

ack

samples/png.cc Outdated
nullptr);
}

std::string image;
Copy link
Collaborator

@dneto0 dneto0 Feb 20, 2019

Choose a reason for hiding this comment

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

Would recommend this instead:
std::string image(png.data(), png.size())

Copy link
Collaborator Author

@dj2 dj2 Feb 20, 2019

Choose a reason for hiding this comment

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

std::string(png.begin(), png.end()) works

@dj2 dj2 merged commit ec5f9d4 into google:master Feb 20, 2019
@dj2 dj2 deleted the png branch February 20, 2019 20:47
@dneto0 dneto0 mentioned this pull request Feb 20, 2019
Closed
dneto0 added a commit to dneto0/amber that referenced this pull request Dec 8, 2022
@dneto0
Roll third_party/spirv-headers/ 0bcc62492..1d31a1004 (22 commits)
6868a2d 
KhronosGroup/SPIRV-Headers@0bcc624...1d31a10

$ git log 0bcc62492..1d31a1004 --date=short --no-merges --format='%ad %ae %s'
2022-12-01 admin Update spir-v.xml
2022-12-01 admin Update spir-v.xml
2022-11-25 stephen.clarke Update spir-v.xml
2022-11-17 admin Register Taichi as SPIR-V generator
2022-11-16 dmitry.sidorov Remove unnecessary extensions addition
2022-11-14 alele Remove extension
2022-11-07 alele Fix typo.
2022-11-07 alele Review feedback 1.
2022-11-04 dneto Revert "Merge pull request google#301 from alan-baker/hpp1" (google#302)
2022-11-03 alanbaker Add spirv.hpp11 to BUILD.gn
2022-10-05 alele Add headers for SPV_NV_shader_invocation_reorder.
2022-10-17 dmitry.sidorov Add SPV_INTEL_runtime_aligned
2022-10-17 dmitry.sidorov Add SPV_INTEL_fpga_dsp_control
2022-10-17 dmitry.sidorov Add SPV_INTEL_fpga_invocation_pipelining_attributes
2022-10-17 dmitry.sidorov Update SPV_INTEL_fpga_loop_controls to rev I
2022-10-05 dkoch Fix extension for RayTracingOpacityMicromapEXT
2022-10-04 kpet Add support for texel buffers to clspv non-semantic instruction set
2022-10-04 hugo registered ID for Shady
2022-08-23 ewerness VK_EXT_opacity_micromap
2021-11-29 kevin.petit Add definitions for SPV_ARM_core_builtins
2021-08-23 pmistry Implement headers definitions for SPV_EXT_mesh_shader.
2022-07-27 jeremy Remove custom target if skipping install

Created with:
  roll-dep third_party/spirv-headers
dneto0 added a commit to dneto0/amber that referenced this pull request Dec 9, 2022
@dneto0
Roll third_party/spirv-headers/ 0bcc62492..1d31a1004 (22 commits)
71210f2 
KhronosGroup/SPIRV-Headers@0bcc624...1d31a10

$ git log 0bcc62492..1d31a1004 --date=short --no-merges --format='%ad %ae %s'
2022-12-01 admin Update spir-v.xml
2022-12-01 admin Update spir-v.xml
2022-11-25 stephen.clarke Update spir-v.xml
2022-11-17 admin Register Taichi as SPIR-V generator
2022-11-16 dmitry.sidorov Remove unnecessary extensions addition
2022-11-14 alele Remove extension
2022-11-07 alele Fix typo.
2022-11-07 alele Review feedback 1.
2022-11-04 dneto Revert "Merge pull request google#301 from alan-baker/hpp1" (google#302)
2022-11-03 alanbaker Add spirv.hpp11 to BUILD.gn
2022-10-05 alele Add headers for SPV_NV_shader_invocation_reorder.
2022-10-17 dmitry.sidorov Add SPV_INTEL_runtime_aligned
2022-10-17 dmitry.sidorov Add SPV_INTEL_fpga_dsp_control
2022-10-17 dmitry.sidorov Add SPV_INTEL_fpga_invocation_pipelining_attributes
2022-10-17 dmitry.sidorov Update SPV_INTEL_fpga_loop_controls to rev I
2022-10-05 dkoch Fix extension for RayTracingOpacityMicromapEXT
2022-10-04 kpet Add support for texel buffers to clspv non-semantic instruction set
2022-10-04 hugo registered ID for Shady
2022-08-23 ewerness VK_EXT_opacity_micromap
2021-11-29 kevin.petit Add definitions for SPV_ARM_core_builtins
2021-08-23 pmistry Implement headers definitions for SPV_EXT_mesh_shader.
2022-07-27 jeremy Remove custom target if skipping install

Created with:
  roll-dep third_party/spirv-headers
dneto0 added a commit to dneto0/amber that referenced this pull request Dec 10, 2022
@dneto0
Roll third_party/spirv-headers/ 0bcc62492..1d31a1004 (22 commits)
9e414aa 
KhronosGroup/SPIRV-Headers@0bcc624...1d31a10

$ git log 0bcc62492..1d31a1004 --date=short --no-merges --format='%ad %ae %s'
2022-12-01 admin Update spir-v.xml
2022-12-01 admin Update spir-v.xml
2022-11-25 stephen.clarke Update spir-v.xml
2022-11-17 admin Register Taichi as SPIR-V generator
2022-11-16 dmitry.sidorov Remove unnecessary extensions addition
2022-11-14 alele Remove extension
2022-11-07 alele Fix typo.
2022-11-07 alele Review feedback 1.
2022-11-04 dneto Revert "Merge pull request google#301 from alan-baker/hpp1" (google#302)
2022-11-03 alanbaker Add spirv.hpp11 to BUILD.gn
2022-10-05 alele Add headers for SPV_NV_shader_invocation_reorder.
2022-10-17 dmitry.sidorov Add SPV_INTEL_runtime_aligned
2022-10-17 dmitry.sidorov Add SPV_INTEL_fpga_dsp_control
2022-10-17 dmitry.sidorov Add SPV_INTEL_fpga_invocation_pipelining_attributes
2022-10-17 dmitry.sidorov Update SPV_INTEL_fpga_loop_controls to rev I
2022-10-05 dkoch Fix extension for RayTracingOpacityMicromapEXT
2022-10-04 kpet Add support for texel buffers to clspv non-semantic instruction set
2022-10-04 hugo registered ID for Shady
2022-08-23 ewerness VK_EXT_opacity_micromap
2021-11-29 kevin.petit Add definitions for SPV_ARM_core_builtins
2021-08-23 pmistry Implement headers definitions for SPV_EXT_mesh_shader.
2022-07-27 jeremy Remove custom target if skipping install

Created with:
  roll-dep third_party/spirv-headers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dneto0 dneto0 dneto0 approved these changes

Assignees

@dj2 dj2

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dj2 @dneto0