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

Image assets have lost detail #3896

Open
GravisZro opened this issue Jun 3, 2022 · 7 comments
Open

Image assets have lost detail #3896

GravisZro opened this issue Jun 3, 2022 · 7 comments

Comments

@GravisZro
Copy link
Contributor

GravisZro commented Jun 3, 2022

🐛 Bug report

📝 Description

The tools used to generate the VPI files are discarding details from the source PNG images. Specifically, the colors have not been quantized which results in loss of detail.

👍 Expected behavior

I expected the source image color palette to be quantized so that values like #0D0D0D are converted to 01.

👎 Current behavior

As far as I can tell, when converting from 24-bit RGB in the PNG the bottom 20-bits are simply discarded.

🔬 Minimal reproduction

This is a step-by-step process used to dump PNG pixel values using nothing but normal tools:
The extra ubuntu/debian packages required are bsdextrautils (for hexdump) and netpbm (for pngtopam).

  • The first two lines set up P as the hexdump output format string. It's just a 16-bit address followed by 32 groups of 3-byte RGB data (i.e. "xxxx | RRGGBB (repeat RRGGBB 31 more times)").
  • The PNG file, plus_32px_W_G.png is printed to standard output
  • pngtopam reads the PNG stream and prints out the image as a binary 24-bit Portable PixMap image. No colors are manipulated in any way, they are written as is. (Note: If you add -plain to the command it will print out as an ASCII 24-bit Portable PixMap image which is a very human readable format.)
  • tail only takes the last line to remove the header data which states the dimensions and image type
  • hexdump convert to the binary data stream to ASCII, printing in a format specified in P
  • The very wide hex dump (232 characters wide) processed with sed with 3 regular expression to make the output readable.
    • Capture two hex digits (RR) by four other hex digits. Replace all six with just the RR hex digits since it's really an grayscale image.
    • Replace all instances of "FF" with spaces (don't clutter output with white)
    • This last expression is used to color the 0D characters (which didn't copy over)
MuditaOSPublicAssets$ P='/1 "%02X"/1 "%02X"/1 "%02X "' \
 P='"%04_ax | "'$P$P$P$P$P$P$P$P$P$P$P$P$P$P$P$P$P$P$P$P$P$P$P$P$P$P$P$P$P$P$P$P'"\n"'; \
 cat source/assets/images/pure/common/plus_32px_W_G.png | \
 pngtopam | \
 tail -n 1 | \
 hexdump -v -e "$P" | \
 sed -e "s|\([[:xdigit:]]\{2\}\)[[:xdigit:]]\{4\}|\1|g" \
     -e "s|FF|  |g" \
     -e "s|0D|$(tput setaf 1)&$(tput sgr0)|g"
0000 |                                                                                                 
0060 |                                                                                                 
00c0 |                                                                                                 
0120 |                                                                                                 
0180 |                                           BD 0D 0D BD                                           
01e0 |                                           7F 00 00 7F                                           
0240 |                                           7F 00 00 7F                                           
02a0 |                                           7F 00 00 7F                                           
0300 |                                           7F 00 00 7F                                           
0360 |                                           7F 00 00 7F                                           
03c0 |                                           7F 00 00 7F                                           
0420 |                                           7F 00 00 7F                                           
0480 |                                           7F 00 00 7F                                           
04e0 |                                           7F 00 00 7F                                           
0540 |             BD 7F 7F 7F 7F 7F 7F 7F 7F 7F 3F 00 00 3F 7F 7F 7F 7F 7F 7F 7F 7F 7F BD             
05a0 |             0D 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0D             
0600 |             0D 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0D             
0660 |             BD 7F 7F 7F 7F 7F 7F 7F 7F 7F 3F 00 00 3F 7F 7F 7F 7F 7F 7F 7F 7F 7F BD             
06c0 |                                           7F 00 00 7F                                           
0720 |                                           7F 00 00 7F                                           
0780 |                                           7F 00 00 7F                                           
07e0 |                                           7F 00 00 7F                                           
0840 |                                           7F 00 00 7F                                           
08a0 |                                           7F 00 00 7F                                           
0900 |                                           7F 00 00 7F                                           
0960 |                                           7F 00 00 7F                                           
09c0 |                                           7F 00 00 7F                                           
0a20 |                                           BD 0D 0D BD                                           
0a80 |                                                                                                 
0ae0 |                                                                                                 
0b40 |                                                                                                 
0ba0 |                                                                                                 

So here we see that the point of each axis is BD 0D 0D BD. When the colors are quantized to 4-bit, they should be translated to 0B 01 01 0B.

So all we need to do is find out the value of the pixels on the fifth row in the VPI image.
I'll point out how the VPI is decoded using a hexdump of the VPI file itself.

  • This sets up P as the hexdump output format string. It's just a 16-bit address followed 8 bytes, a space, and 8 more bytes.
  • hexdump of plus_32px_W_G.vpi with output format specified in $P
MuditaOSPublicAssets$ P='"%04_ax | " 8/1 "%02X " "  " 8/1 "%02X " "\n"'; \
 hexdump -e "$P" image/assets/images/plus_32px_W_G.vpi
0000 | 20 00 20 00 10 01 00 00  00 20 0F 01 00 00 00 20
0010 | 0F 01 00 00 00 20 0F 01  00 00 00 20 0F 05 00 00
0020 | 00 0E 0F 00 00 01 0B 00  00 02 00 00 00 01 0B 00

  • Header data
    • 0x0000 - two bytes: width: 32
    • 0x0002 - two bytes: height: 32
    • 0x0004 - one byte: transparency: 0x10
  • 0x0005 - two bytes: Line 1 vector count : 1 vector
    • 0x0007 - 1 vector * 4 bytes per vector (ignored)
  • 0x000C - two bytes: Line 2 vector count : 1 vector
    • 0x000E - 1 vector * 4 bytes per vector (ignored)
  • 0x0011 - two bytes: Line 3 vector count : 1 vector
    • 0x0013 - 1 vector * 4 bytes per vector (ignored)
  • 0x0017 - two bytes: Line 4 vector count : 1 vector
    • 0x0019 - 1 vector * 4 bytes per vector (ignored)
  • 0x001D - two bytes: Line 5 vector count : 5 vectors
    • vector 0
      • 0x001F - two bytes: vector offset: 0
      • 0x0021 - one byte: vector run-length: 14 pixels (0x0E)
      • 0x0022 - one byte: vector color: 0x0F
    • vector 1
      • 0x0023 - two bytes: vector offset: 0
      • 0x0025 - one byte: vector run-length: 1 pixel
      • 0x0026 - one byte: vector color: 0x0B
    • vector 2 (target pixels)
      • 0x0027 - two bytes: vector offset: 0
      • 0x0029 - one byte: vector run-length: 2 pixels (0x02)
      • 0x002A - one byte: vector color: 0x00 (Detail lost! Expected 0x01)
    • vector 3
      • 0x002B - two bytes: vector offset: 0
      • 0x002D - one byte: vector run-length: 1 pixel
      • 0x002E - one byte: vector color: 0x0B

So manually decoding the data shows that the pixels that should be 0B 01 01 0B are actually 0B 00 00 0B. This is a loss of detail.

@pholat
Copy link
Contributor

pholat commented Jun 6, 2022

@GravisZro image converter is not part of the MuditaOS. Therefore it's not a bug to the OS.

@pholat pholat closed this as completed Jun 6, 2022
@pholat
Copy link
Contributor

pholat commented Jun 6, 2022

Saying that - I believe you are right that the software used is lacking in this aspect. (Not OSS right now, link for reference for people with access)
Do you propose to change how we generate images, opensource the simple soft creating MPI & VPI, other?

@pholat pholat reopened this Jun 6, 2022
@GravisZro
Copy link
Contributor Author

I would like to request that you cease being dismissive. I have tried to bring this up before but you closed the issue and ignored my request to reopen the issue. It is extremely irritating to put in the effort to report and fix flaws in the project only to be ignored and dismissed. Neglecting community efforts is self-defeating.

With this issue in particular, I explicitly stated in the description of this issue (and the previous issue) that the cause of this loss of detail is a lack of color quantization.

@pholat
Copy link
Contributor

pholat commented Jun 6, 2022

This actually hurt, as we lost quite a lot of time reaching out to you, I believe this is the origin of this particular issue, where you at the beginning introduced code that was crashing - into the perfectly working codebase (for the current needs), which was found out by unit tests you denied writing. So first and foremost - please do your best to help us out. It's not our goal to provide support and have nothing from it.

Saying that I'm sorry to hear that you feel that way, it was not my intention. I actually am very fond of community, when the question arose "shall we OSS" I always advocate for yes, and always promote that we open how much we can. I understand that your findings feel the most important to you, but to us, it's just a tiny bit of scope.

Please drop the personal remarks as it's not getting us anywhere, be as supportive as you can and help us out instead of adding weight on our backs.

@GravisZro
Copy link
Contributor Author

I recognize that a sizeable chunk of the time that has been invested in accommodating community members, myself especially and I'm grateful for it. As for the unit testing, I would gladly contribute tests if I had a better insight into the codebase as well as catch2 and I'm continuing to work on these.

I'm quite glad you to have an advocate for open source in Mudita and I hope you continue pushing for it despite any setbacks.

I apologize as my intent was not to harm but it seems my frustrations/concerns were improperly presented which is why I opened multiple github issues to present them properly so they may be addressed in a neutral manner.

@kkleczkowski
Copy link
Contributor

@GravisZro It's being internally processed right now. Repo will be opened in the nearest future.

@SP2FET
Copy link
Contributor

SP2FET commented Jul 25, 2022

Hi @GravisZro ,
As you requested, we opened the PureImageConverter repository: https://github.com/mudita/PureImageConverter

Feel free to contribute :)

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

No branches or pull requests

4 participants