#240 correct alignment for 24x24 icons

closed-fixed
tklib: ico (2)
5
2010-07-07
2010-05-26
Anton Kovalenko
No

When tklib::ico reads and writes *.ico, *.dll, *.exe files, it uses the code like ($w*$h+$h*($w%32)) in a number of places, to calculate extra padding required for AND mask. It turned out that the result for 24x24 icons is incorrent; when used with this size, tklib::ico may corrupt executable files and generate incorrect *.ico files (still accepted by some software, but e.g. icotool warns about the incorrect size and about garbage skipped).

If we regard $w==24 as a special case, ($w==24? 8 : $w % 32) is a correct replacement for the expression above. I'm attaching the patch fixing all places where the incorrect method was used.

Wish.ico has 24x24 members, and patched ico.tcl works fine with wish-based executables.

Discussion

  • fix alignment for 24x24 icons

     
    Attachments
    • assigned_to: afaupell --> andreas_kupries
     
  • The patch contains a number of other changes beyond what is described in this bug.
    Can you explain ? (Changes to binary scan, file handling, int/float differences).

     
  • Umm, sorry, almost all of them are safely ignored, many are redundant and many depends on newer Tcl than ico.tcl itself. Sorry again, I should have prepared a cleaner patch.

    Float->int replacement doesn't alter semantics for icon dimensions that are multiples of 8. It was added during a vain attempt to track down the size calculation problem. The same can be said about [open] flags ("b" is redundant as soon as -encoding and -translation are set in a later code) and [binary scan] ("u" flag is more convenient than format %u and &0xFFFF, but, alas, it increases required Tcl version).

    The only thing making sense, of those other changes, is commenting out a check for [file exists] in writeIcon: ICO backend is able to create files from scratch, so the file doesn't _have_ to exist (I should report it as a separate bug and attach a separate patch, of course).

    As of other changes, please drop them out.

     
  • Reduced and extended patch (changelog, package index, docs) as committed

     
    Attachments
    • status: open --> closed-fixed
     
  • Ok. Done. Version is 1.0.5. See my attachment for the final patch I committed to CVS head. Please check that it works for you. If not, reopen the bug.