240325_wxrc_FileToCppArray_01h (PR #24437)

74 views
Skip to first unread message

DoctorNoobingstoneIPresume

unread,
Mar 25, 2024, 3:27:26 AMMar 25
to wx-...@googlegroups.com, Subscribed

wxrc: FileToCppArray and MakePackageCPP:

  • We no longer claim to support resources larger than 4 GiB.
  • We have replaced new [] and delete [] with std::vector <...>.
  • Temp file info (index, size, name) is now printed before defining each byte-array.
  • static has been replaced by anonymous namespace. const has been added.
  • Byte-array initialization integer literals are now hex; progress is shown.

This pull request replaces part of #24413 (as a sequence of more focused commits).

Thank you for considering this and for kind comments !! 🤗


You can view, comment on, or merge this pull request online at:

  https://github.com/wxWidgets/wxWidgets/pull/24437

Commit Summary

  • a48892d Expose reason for notification dismissal through SetInt()
  • a9575c3 Always post notification dismissal event on Gtk, regardless of reason
  • 933870c Fix calling wxOverlay::Reset() without Init() first under Mac
  • bfacc1d wxrc: `FileToCppArray`: We no longer claim to support resources larger than 4 GiB.
  • 7b3bc15 wxrc: `FileToCppArray`: We have replaced `new []` and `delete []` with `std::vector <...>`.
  • ee58d9e wxrc: `FileToCppArray`: Temp file info (index, size, name) is now printed before defining each byte-array.
  • f38ce73 wxrc: `FileToCppArray`: `static` has been replaced by anonymous namespace. `const` has been added.
  • 33c983d wxrc: `FileToCppArray`: Byte-array initialization integer literals are now hex; progress is shown.

File Changes

(10 files)

Patch Links:


Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437@github.com>

DoctorNoobingstoneIPresume

unread,
Mar 25, 2024, 6:00:44 AMMar 25
to wx-...@googlegroups.com, Subscribed

I apologize. I am not sure why 8 commits are included in my pull request. Only 5 of them have been intended to be included... :'( I am going to look into it...


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437/c2017626735@github.com>

DoctorNoobingstoneIPresume

unread,
Mar 26, 2024, 5:00:18 AMMar 26
to wx-...@googlegroups.com, Push

@DoctorNoobingstoneIPresume pushed 2 commits.

  • 6fb6764 wxrc: `GetInternalFileName`: Call to `Printf`: Format string arg is now constant.
  • e822473 wxrc: `MakePackageCPP`: Faster&leaner compilation of C++ source code generated by our resource compiler.


View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437/push/17706738068@github.com>

DoctorNoobingstoneIPresume

unread,
Mar 26, 2024, 5:03:30 AMMar 26
to wx-...@googlegroups.com, Subscribed

The following commit has now been added, beyond the scope of #24413:

  • wxrc: GetInternalFileName: Call to Printf: Format string arg is now constant.

The following commit has now also been added, completing (in my opinion) the replacement of #24413:

  • wxrc: MakePackageCPP: Faster&leaner compilation of C++ source code generated by our resource compiler.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437/c2019873577@github.com>

DoctorNoobingstoneIPresume

unread,
Mar 27, 2024, 9:39:33 AMMar 27
to wx-...@googlegroups.com, Push

@DoctorNoobingstoneIPresume pushed 1 commit.

  • 524b53d wxrc: `MakePackageCPP`: Faster&leaner compilation of C++ source code generated by our resource compiler.


View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437/push/17729282637@github.com>

VZ

unread,
Apr 8, 2024, 2:10:08 PMApr 8
to wx-...@googlegroups.com, Subscribed

@vadz commented on this pull request.

Thanks for the update!

I think I'd rather move the (useful, thank you) explanations of the changes to the commit messages and leave just the brief summary in the comments in the code, but I can do it myself when merging this.

The biggest remaining question is whether we want the extra complexity of dealing with the sections in the output. IMO this is not really useful and I don't understand why would anybody want to look at these files, they're really meant to be only used as an input to the compiler and never read by a human. So I'd like to exclude the commits changing this when merging.

But I'd like to know what do the others think about it, would anybody else find it useful to have the section headers in the generated output?


In utils/wxrc/GenerateBigXRCFile.sh:

> +# (when thousands of input resource files have been given as input).
+
+GenerateExampleImages ()
+{
+    local folder_images='ExampleImages'
+    mkdir -p "${folder_images}/"
+
+    local i
+    local n="${1:-10000}"
+
+    printf '<?xml version="1.0" encoding="UTF-8"?>\n'
+    printf '<resource xmlns="http://www.wxwidgets.org/wxxrc" version="2.5.3.0">\n'
+    for ((i = 0; i < n; ++i)); do
+        local pathname_image; printf -v pathname_image "%s/Example_%04Xh.png" "${folder_images}" "$((i))"
+
+        if ((1)); then

Sorry, what's the purpose of this line?


In utils/wxrc/GenerateBigXRCFile.sh:

> +            printf >>"${pathname_image}" "\x89\x50\x4E\x47\x0D\x0A\x1A\x0A\x00\x00\x00\x0D\x49\x48\x44\x52"
+            printf >>"${pathname_image}" "\x00\x00\x00\x10\x00\x00\x00\x10\x08\x02\x00\x00\x00\x90\x91\x68"
+            printf >>"${pathname_image}" "\x36\x00\x00\x00\x09\x70\x48\x59\x73\x00\x00\x0E\xC4\x00\x00\x0E"
+            printf >>"${pathname_image}" "\xC4\x01\x95\x2B\x0E\x1B\x00\x00\x00\x1A\x49\x44\x41\x54\x28\xCF"
+            printf >>"${pathname_image}" "\x63\x6C\x60\xF8\xCF\x40\x0A\x60\x62\x20\x11\x8C\x6A\x18\xD5\x30"
+            printf >>"${pathname_image}" "\x74\x34\x00\x00\xC5\xBF\x01\x9F\x22\x91\xFF\xBD\x00\x00\x00\x00"
+            printf >>"${pathname_image}" "\x49\x45\x4E\x44\xAE\x42\x60\x82"

Do we really have to compose the file piece-wise like this? Why can't we just add it to the repository?


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437/review/1987080578@github.com>

DoctorNoobingstoneIPresume

unread,
Apr 9, 2024, 10:12:12 AMApr 9
to wx-...@googlegroups.com, Subscribed

Thanks for the update!

Thank you for everything !!

I think I'd rather move the (useful, thank you) explanations of the changes to the commit messages and leave just the brief summary in the comments in the code, but I can do it myself when merging this.

Ok, I was thinking that we do not normally have commit messages which are this long, and also to (maybe by force) get the message across to more people 😈 , but either way is fine with me, and I can modify the comment/commit messages and also you please feel free to do it as you think is best.

The biggest remaining question is whether we want the extra complexity of dealing with the sections in the output. IMO this is not really useful and I don't understand why would anybody want to look at these files, they're really meant to be only used as an input to the compiler and never read by a human. So I'd like to exclude the commits changing this when merging.

But I'd like to know what do the others think about it, would anybody else find it useful to have the section headers in the generated output?

My opinion has been in this reply to the previous pull request: #24413 (comment).

For review of the output, here is the XRC output of running './Example_Generator.sh' 100 (i.e. XRC file referencing 100 files):

and here are the .h file and .cpp file generated by processing that XRC file with wxrc:

In the latter file (the .cpp file), we have the C++ array representation of 100 short PNG files and a longer file (the XRC file itself). When scrolling through the representation of the longer file, I think the section headers (every 16 lines, i.e. every 256 bytes) might help.

Also, I think that the C++ array representations of the binary files might be useful for debugging. For example, the author might think she has given an UTF-8 encoded file, and might wonder why she does not see the expected glyphs, and in order to debug she might run hexdump -Cv input_file.txt and compare that with the hex representation in the wxrc-generated C++ code (at the exact offsets where non-ASCII code points are expected) => helping her with offsets might be useful.

However, I am willing to remove these section headers if you and/or the community thinks otherwise, or you can also remove them as you see fit if you will.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437/c2045281033@github.com>

DoctorNoobingstoneIPresume

unread,
Apr 9, 2024, 10:15:26 AMApr 9
to wx-...@googlegroups.com, Subscribed

@DoctorNoobingstoneIPresume commented on this pull request.


In utils/wxrc/GenerateBigXRCFile.sh:

> +            printf >>"${pathname_image}" "\x89\x50\x4E\x47\x0D\x0A\x1A\x0A\x00\x00\x00\x0D\x49\x48\x44\x52"
+            printf >>"${pathname_image}" "\x00\x00\x00\x10\x00\x00\x00\x10\x08\x02\x00\x00\x00\x90\x91\x68"
+            printf >>"${pathname_image}" "\x36\x00\x00\x00\x09\x70\x48\x59\x73\x00\x00\x0E\xC4\x00\x00\x0E"
+            printf >>"${pathname_image}" "\xC4\x01\x95\x2B\x0E\x1B\x00\x00\x00\x1A\x49\x44\x41\x54\x28\xCF"
+            printf >>"${pathname_image}" "\x63\x6C\x60\xF8\xCF\x40\x0A\x60\x62\x20\x11\x8C\x6A\x18\xD5\x30"
+            printf >>"${pathname_image}" "\x74\x34\x00\x00\xC5\xBF\x01\x9F\x22\x91\xFF\xBD\x00\x00\x00\x00"
+            printf >>"${pathname_image}" "\x49\x45\x4E\x44\xAE\x42\x60\x82"

I was thinking we have a completely independent single-file script, but we can of course move the actual content to an external file if you wish.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437/review/1989177875@github.com>

DoctorNoobingstoneIPresume

unread,
Apr 9, 2024, 10:21:19 AMApr 9
to wx-...@googlegroups.com, Push

@DoctorNoobingstoneIPresume pushed 1 commit.

  • 9f5fdc5 wxrc: `MakePackageCPP`: Faster&leaner compilation of C++ source code generated by our resource compiler.


View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437/push/17919507502@github.com>

DoctorNoobingstoneIPresume

unread,
Apr 9, 2024, 10:21:41 AMApr 9
to wx-...@googlegroups.com, Subscribed

@DoctorNoobingstoneIPresume commented on this pull request.


In utils/wxrc/GenerateBigXRCFile.sh:

> +# (when thousands of input resource files have been given as input).
+
+GenerateExampleImages ()
+{
+    local folder_images='ExampleImages'
+    mkdir -p "${folder_images}/"
+
+    local i
+    local n="${1:-10000}"
+
+    printf '<?xml version="1.0" encoding="UTF-8"?>\n'
+    printf '<resource xmlns="http://www.wxwidgets.org/wxxrc" version="2.5.3.0">\n'
+    for ((i = 0; i < n; ++i)); do
+        local pathname_image; printf -v pathname_image "%s/Example_%04Xh.png" "${folder_images}" "$((i))"
+
+        if ((1)); then

Thank you. I have removed it (and the corresponding fi of the if).


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437/review/1989194920@github.com>

DoctorNoobingstoneIPresume

unread,
Apr 10, 2024, 5:32:32 AMApr 10
to wx-...@googlegroups.com, Push

@DoctorNoobingstoneIPresume pushed 3 commits.

  • 73a99c5 wxrc: `FileToCppArray`: Byte-array initialization integer literals are now hex; progress is optionally printed.
  • 9a5d32d wxrc: `GetInternalFileName`: Call to `Printf`: Format string arg is now constant.
  • b8907f2 wxrc: `MakePackageCPP`: Faster&leaner compilation of C++ source code generated by our resource compiler.


View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437/push/17932497681@github.com>

DoctorNoobingstoneIPresume

unread,
Apr 10, 2024, 5:34:50 AMApr 10
to wx-...@googlegroups.com, Subscribed

The biggest remaining question is whether we want the extra complexity of dealing with the sections in the output. IMO this is not really useful and I don't understand why would anybody want to look at these files, they're really meant to be only used as an input to the compiler and never read by a human. So I'd like to exclude the commits changing this when merging.
But I'd like to know what do the others think about it, would anybody else find it useful to have the section headers in the generated output?

Section headers are now optional (via e.g. --dump-section-size=256) and disabled by default:

https://github.com/wxWidgets/wxWidgets/compare/9f5fdc57d3430384c07c20c61d8b37b6be1efac9..b8907f2525e3ca9c9769a2566b1713b086fe5496

😈


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437/c2047029447@github.com>

DoctorNoobingstoneIPresume

unread,
Apr 10, 2024, 5:37:13 AMApr 10
to wx-...@googlegroups.com, Subscribed

I think I'd rather move the (useful, thank you) explanations of the changes to the commit messages and leave just the brief summary in the comments in the code, but I can do it myself when merging this.

Ok, I was thinking that we do not normally have commit messages which are this long, and also to (maybe by force) get the message across to more people 😈 , but either way is fine with me, and I can modify the comment/commit messages and also you please feel free to do it as you think is best.

Perhaps someone wants to later add benchmarks for another toolchain (e.g. clang++) -- then he or she can make a commit which modifies/extends the comments in wxrc.cpp. I think this would be more difficult if the benchmarks are in the commit message...


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437/c2047034012@github.com>

VZ

unread,
Apr 25, 2024, 10:43:59 AMApr 25
to wx-...@googlegroups.com, Subscribed

Sorry for the delay with coming back to it and thanks for splitting this into separate commits. I'd like to make some comments for each of them:

  1. 48c3f2c (wxrc: FileToCppArray: Resource size is now limited to 4 GiB on all platforms.): I agree with the change, but I think we should keep using size_t and just compare that the length of the file is not greater than 4 (or even maybe 2) GiB. I also still think that the explanatory comment could be much shorter and just say that we want the generated file to compile on all systems, including 32 bit ones. Finally, we definitely should use actual error checking and wxLogError() instead of wxASSERT_MSG() because the failure of the condition being checked doesn't indicate a problem in the program logic but one with its inputs.
  2. a3291f8 (wxrc: FileToCppArray: We have replaced new [] and delete [] with std::vector <...>.): No comments, definitely a good change, thanks.
  3. 868c382 (wxrc: FileToCppArray: Temp file info (index, size, name) is now printed before defining each byte-array., 2024-03-25) and 73a99c5 (wxrc: FileToCppArray: Byte-array initialization integer literals are now hex; progress is optionally printed.): Thanks for making this optional but unless someone else votes for having this, I'd still rather omit these commits, they just seem to add more code without any real benefit for 99.99% of wx users.
  4. 4f82001 (wxrc: FileToCppArray: static has been replaced by anonymous namespace. const has been added.): const is good but I see no real advantage of replacing static. Is there any reason to do it?
  5. 9a5d32d (wxrc: GetInternalFileName: Call to Printf: Format string arg is now constant.): Good change, thanks.
  6. b8907f2 (wxrc: MakePackageCPP: Faster&leaner compilation of C++ source code generated by our resource compiler.): Also good change, but I'd rather put most of the explanations into the commit message than in the comment in the source.

To summarize, I'd like to apply (2) and (5) as is, (1) and (4) with minor changes and (6) with just some comment reformatting/moving, while omitting (3).

But I'll wait for a bit to see if anybody else wants to have (3). And, of course, please let me know if you have any comments.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437/c2077394039@github.com>

DoctorNoobingstoneIPresume

unread,
May 20, 2024, 7:40:47 PMMay 20
to wx-...@googlegroups.com, Subscribed

Thank you for kind answer. My partial answer follows:

[...]
1. 48c3f2c (wxrc: FileToCppArray: Resource size is now limited to 4 GiB on all platforms.): I agree with the change, but I think we should keep using size_t and just compare that the length of the file is not greater than 4 (or even maybe 2) GiB. I also still think that the explanatory comment could be much shorter and just say that we want the generated file to compile on all systems, including 32 bit ones. Finally, we definitely should use actual error checking and wxLogError() instead of wxASSERT_MSG() because the failure of the condition being checked doesn't indicate a problem in the program logic but one with its inputs.
[...]

Please kindly see #24546.

[...]
5. 9a5d32d (wxrc: GetInternalFileName: Call to Printf: Format string arg is now constant.): Good change, thanks.
[...]

Please kindly see #24547.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437/c2121408778@github.com>

VZ

unread,
May 23, 2024, 12:04:38 PMMay 23
to wx-...@googlegroups.com, Subscribed

Your PRs corresponding to (1) and (5) have been merged and I've also pushed 3c41a68 which does what (2) do and more.

So, basically, there is just (6) left here.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437/c2125965462@github.com>

DoctorNoobingstoneIPresume

unread,
May 29, 2024, 6:57:19 PMMay 29
to wx-...@googlegroups.com, Subscribed

[...]
6. b8907f2 (wxrc: MakePackageCPP: Faster&leaner compilation of C++ source code generated by our resource compiler.): Also good change, but I'd rather put most of the explanations into the commit message than in the comment in the source.
[...]

Please kindly see #24579...


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437/c2138386223@github.com>

VZ

unread,
Jun 9, 2024, 6:25:49 AM (7 days ago) Jun 9
to wx-...@googlegroups.com, Subscribed

Closed #24437.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437/issue_event/13090692696@github.com>

VZ

unread,
Jun 9, 2024, 6:25:50 AM (7 days ago) Jun 9
to wx-...@googlegroups.com, Subscribed

I think there is nothing remaining to merge here any longer, so closing. Thanks!


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/24437/c2156431806@github.com>

Reply all
Reply to author
Forward
0 new messages