240325_wxrc_FileToCppArray_01h (PR #24437)
DoctorNoobingstoneIPresume
wxrc: FileToCppArray and MakePackageCPP:
- We no longer claim to support resources larger than 4 GiB.
- We have replaced
new []anddelete []withstd::vector <...>. - Temp file info (index, size, name) is now printed before defining each byte-array.
statichas been replaced by anonymous namespace.consthas 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
- M include/wx/notifmsg.h (9)
- M interface/wx/notifmsg.h (18)
- M samples/dialogs/dialogs.cpp (3)
- M src/generic/notifmsgg.cpp (3)
- M src/gtk/notifmsg.cpp (19)
- M src/msw/notifmsg.cpp (2)
- M src/msw/rt/notifmsgrt.cpp (20)
- M src/osx/cocoa/notifmsg.mm (2)
- M src/osx/cocoa/overlay.mm (14)
- M utils/wxrc/wxrc.cpp (116)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/24437.patch
- https://github.com/wxWidgets/wxWidgets/pull/24437.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
DoctorNoobingstoneIPresume
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.
DoctorNoobingstoneIPresume
@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.
DoctorNoobingstoneIPresume
The following commit has now been added, beyond the scope of #24413:
- wxrc:
GetInternalFileName: Call toPrintf: 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.
DoctorNoobingstoneIPresume
@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.
VZ
@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.
DoctorNoobingstoneIPresume
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.
DoctorNoobingstoneIPresume
@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.
DoctorNoobingstoneIPresume
@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.
DoctorNoobingstoneIPresume
> +# (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.
DoctorNoobingstoneIPresume
@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.
DoctorNoobingstoneIPresume
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:
😈
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
DoctorNoobingstoneIPresume
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.
VZ
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:
- 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 usingsize_tand 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 andwxLogError()instead ofwxASSERT_MSG()because the failure of the condition being checked doesn't indicate a problem in the program logic but one with its inputs. - a3291f8 (wxrc:
FileToCppArray: We have replacednew []anddelete []withstd::vector <...>.): No comments, definitely a good change, thanks. - 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. - 4f82001 (wxrc:
FileToCppArray:statichas been replaced by anonymous namespace.consthas been added.):constis good but I see no real advantage of replacing static. Is there any reason to do it? - 9a5d32d (wxrc:
GetInternalFileName: Call toPrintf: Format string arg is now constant.): Good change, thanks. - 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.
DoctorNoobingstoneIPresume
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 usingsize_tand 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 andwxLogError()instead ofwxASSERT_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 toPrintf: 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.
VZ
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.
DoctorNoobingstoneIPresume
[...]
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.
VZ
Closed #24437.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
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.
DoctorNoobingstoneIPresume
Thank you for support and also for the confidence -- and also for considering even my preferences -- I could see that you have included the benchmarks as C++ comments... thank you so much !! Also, my modest attempts with wxWidgets have brought me happiness during the last few weeks... 😹
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Thanks again for your contribution and, of course, the preferences of the person doing the actual work count, so it's perfectly normal to consider them!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.