#213 Modernised icon set for menus and toolbars

[Vidar Hasfjord]

Currently the library includes a small set of bitmaps for use with TControlBar as TButtonGadget glyphs and with menus for menu item bitmaps (see TMenu::SetMenuItemBitmaps). The bitmaps cover icons for standard file operation, edit operations, search, preview, help — basically the bitmaps included in the original OWL examples, such as RichEditor.

See "include/owl/res" for the bitmap files included. None have been updated since [r5], it seems.

The quality of these old 4-bit glyphs is poor, and the style is pretty dated. Now that we have support for 24-bit bitmaps in TButtonGadget [feature-requests:#212], I propose we update our icon set!

PS. By the way, OWL 5 does not include these bitmaps in the same location. Most are found as part of OWL AppExpert in "expert/owl", while some of the preview toolbar bitmaps are in-lined within "include/owl/prevwin.rc", slider bitmaps are in-lined in "include/owl/slider.rc", and the application icons are in-lined in "include/owl/owlapp.rc".

[Vidar Hasfjord]

A new icon set has now been committed on the trunk [r6124], and it has been merged into Owlet [r6125].

Note the new icon set is still incomplete, and old bitmaps remain to be updated, in particular for editing, printing and preview.

[Vidar Hasfjord]

Hi Ognyan,

Should we convert the few standard ones that are still 4-bit to 24-bit, like print.bmp?

I see no need to just convert the files if the 4-bit colour versions still work fine (which I hope they do — I just tested RichEditor Example and it seems fine).

That said, while removing superfluous bitmaps in RichEditor (duplicates of bitmaps in "include/owl/res") [r6203], I noticed that "include/owl/res/preview.bmp" (4-bit colour) shows up as garbage. I repaired it by simply resaving it in the Visual Studio Resource Editor, after which it shows up fine [r6202]. There may be more old bitmaps in "include/owl/res" with the same problem.

As part of my work on this ticket (for use with OWLMaker), I have updated many of the toolbar icon bitmaps in "include/owl/res", including "open.bmp", "save.bmp", etc. [r6124]. These are new 24-bit bitmaps based on LibreOffice's default icon set. So if you want to update the remaining ones ("print.bmp" etc.), please use the same style. That was my plan, anyway. If you have a better icon set we can freely use, that would be great.

By the way, the preview close bitmap "prexit.bmp" is just a text ("Close"), and I think we should replace it by a close icon (x). Then we can remove the Japanese variant at the same time ("prexitj.bmp").

[Ognyan Chernokozhev]

These are new 24-bit bitmaps based on LibreOffice's default icon set.

These new bitmaps look great!

By the way, the preview close bitmap "prexit.bmp" is just a text ("Close"), and I think we should replace it by a close icon (x).

Yes, there is no sense of having a bitmap with just printed text, It should be either an icon, or a text button.

[Vidar Hasfjord]

CHG: Owlet: Overhauled print and preview icons [r8855].
CHG: Owlet: New more descriptive "mdi-close" icon [r8856].

@jogybl wrote:

Yes, [replace "prexit.bmp", as it makes no sense] having a bitmap with just printed text

Done; "prexit.bmp" is now Ⓧ (a red circled X);, same as LibreOffice uses for closing its print preview. Obsolete "prexitj.bmp" (Japanese) has been removed.

Feel free to merge these changes into the trunk.

[Ognyan Chernokozhev]

Strange - when using the new bitmaps in a control bar, they show as garbage.

In the menus they show fine.

[Ognyan Chernokozhev]

One difference that I see is that for the new bitmpas, in the BITMAPINFOHEADER the members biXPelsPerMeter and biYPelsPerMeter are set to 3780, while for the other bitmaps, that show fine in the toolbar, they are 0.

[Ognyan Chernokozhev]

Ok, problem identified. The core issue is that the biSizeImage member of BITMAPINFOHEADER struct is set to 0. Which, according to the documentation, is fine for uncompressed RGB bitmaps.

But the TDib class does not handle that well. As a matter of fact, when loading a bitmap from a file, there is code that calculates the size. But when the bitmap is loaded from a resource, that code is not executed, and the size is zero. I suspect the issue may be in the ResToMemHandle method, where the bitmap is copied, and the memory size is calculated using that member - and if it is 0, then the bitmap may not be copied correctly.

Changing the calculation to use the same size correction seem to solve the problem.

[Ognyan Chernokozhev]

Documented in [bugs:#650].

[Vidar Hasfjord]

@jogybl wrote:

Strange - when using the new bitmaps in a control bar, they show as garbage. [...] Ok, problem identified. The core issue is that the biSizeImage member of BITMAPINFOHEADER struct is set to 0. As a matter of fact, when loading a bitmap from a file, there is code that calculates the size. But when the bitmap is loaded from a resource, that code is not executed, and the size is zero.

Thanks for the debugging! I think this is a long-standing issue that I've encountered many times before, in which cases the workaround has been to resave in Visual Studio. It is great to have this issue fully diagnosed and fixed. Kudos!

The reason I didn't encounter this issue in the latest version of Owlet is due to my major refactoring work. TGadgetButton no longer uses TDib, nor TCelArray. Instead, it loads the TBitmap in the constructor, using the the TBitmap constructor that loads directly from resource. Hence, the bug is circumvented.

In Owlet, I've also made a major overhaul of TDib. But it is still based on the old code, and as such I have probably carried over the bug. I will review and apply your fix, if necessary.

Edit: The bug fix was needed and has now been merged into Owlet [r8859]. Old-style casts replaced and precondition assertions added in [r8860].