Opened 5 years ago

Closed 5 years ago

#12207 closed enhancement (fixed)

Add Tool to Render HVIFs to PNG

Reported by: apl-haiku Owned by: apl-haiku
Priority: normal Milestone: Unscheduled
Component: Build System Version: R1/Development
Keywords: hvif Cc:
Blocked By: Blocking:
Platform: All


As discussed on the HDS mailing list, this new tool is required for the HDS application server to be able to render HVIF files instead of have manually rendered PNG icons uploaded separately from the HVIFs. It has to be able to be executed on the linux platform. Andrew (me) is actively working on this.

Attachments (3)

Change History (12)

comment:1 by apl-haiku, 5 years ago

patch: 01

comment:2 by axeld, 5 years ago

It doesn't follow our coding style at all, but I guess that's not entirely important for this tool. Otherwise, the patch looks good from a build system perspective (I have not tested the tool itself, though).

comment:3 by waddlesplash, 5 years ago

I'll clean up the coding style, if Andrew doesn't have the time / isn't familiar with it.

comment:4 by apl-haiku, 5 years ago

Hi @waddlesplash & @axeld. I am keen to follow those coding styles. I did run the python script and thought that it would have picked up on "most problems", but obviously there's more! I will take a look at those in the next couple of days and see if I can get it into line.

comment:5 by bonefish, 5 years ago

To point out the coding style issues:

  • src/tools/Jamfile: The SubIncludes are sorted alphabetically.
  • The '#include's in each group are sorted alphabetically.
  • The preferred pointer/reference style is FILE* stream over FILE *stream.
  • No space after cast operator.
  • if/while conditions should be foo == <constant> rather than <constant> == foo.
  • When an if/else statement is only a single line (note: not just a single statement), the block braces should be omitted.
  • The maximum number of character columns per line is 80 (tab stops at multiples of 4 columns). Anything longer should be wrapped.
  • The { of a function block should go on a new line (the if, for, etc. blocks are fine).
  • Local variables:
    • Should be lower camel case (aka dromedary case) instead of using underscores.
    • png_ptr, info_ptr should simply be named png and info (or pngInfo) respectively.
    • If possible, variable definitions and initializations should be combined.
  • The comments above functions should be converted to doxygen style comments. There's an example in the coding guidelines (search for "/*!")
  • Mostly concerning h2p_write_png(): Instead of nesting ifs and elses, the early-error-return style (if (<error>) return ...;) is preferred for better readability. In C-style code that leads to duplication of deinitialization code. In this case it would be minimal, but if you like you can use RAII helpers -- namely CObjectDeleter -- from AutoDeleter.h to avoid it altogether. Usage examples: destructor function returning void, destructor function returning non-void (explicit namespace denotation not necessary).
  • The use of blank lines is a bit excessive. While their use is encouraged for separating groups of statements, blank lines at the beginning of blocks, after case ...: lines, or within a sequence of }s should be omitted.

comment:6 by apl-haiku, 5 years ago

Hello bonefish; thank you for taking the time to go through that patch. I've not used the "RAII" mechanism before so that was quite interesting. When you get a moment, could you please have a look at my updated patch and see if my usage is correct and if you can see any other anomalies. Regards.

comment:7 by bonefish, 5 years ago

Patch applied in hrev49413. I adjusted the commit message a bit, though. In separate commits I fixed the remaining coding style issues and also an issue with closing the streams.

BTW, I tried the tool with several icons from data/artwork/icons and it seems to run into an infinite loop.

comment:8 by apl-haiku, 5 years ago

@bonefish; It looks like those files are not HVIF. HVIF has a magic string of "ncif", but those files seem to have a leading 32 bit word of "IMSG". I've created a new patch so that the tool can sniff the file first and also fixed the infinite-looping issue too - thanks.

comment:9 by bonefish, 5 years ago

Resolution: fixed
Status: newclosed

Andrew, you're right, those files are Icon-O-Matic document files, not HVIF files.

Applied you patch in hrev49417 with small coding style fixes (blank line, commit message).

Note: See TracTickets for help on using tickets.