#240 correct alignment for 24x24 icons

closed-fixed
tklib: ico (2)
5
2010-07-07
2010-05-26
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

  • Anton Kovalenko

    Anton Kovalenko - 2010-05-26

    fix alignment for 24x24 icons

     
  • Andreas Kupries

    Andreas Kupries - 2010-07-06
    • assigned_to: afaupell --> andreas_kupries
     
  • Andreas Kupries

    Andreas Kupries - 2010-07-06

    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).

     
  • Anton Kovalenko

    Anton Kovalenko - 2010-07-06

    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.

     
  • Andreas Kupries

    Andreas Kupries - 2010-07-07

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

     
  • Andreas Kupries

    Andreas Kupries - 2010-07-07
    • status: open --> closed-fixed
     
  • Andreas Kupries

    Andreas Kupries - 2010-07-07

    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.

     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks