Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#10635 closed enhancement (fixed)

wxOwnerDrawn refactoring

Reported by: malcompl Owned by:
Priority: normal Milestone:
Component: GUI-all Version: stable-latest
Keywords: wxOwnerDrawn Cc:
Blocked By: Blocking:
Patch: yes

Description

Refactore wxOwnerDrawn class for avoid specific stuff in base class, like operations and drawing bitmaps.

For more info see discussion about it on wx-dev group.

Some known problem to resolve in new patch:

  • appearance and improvement wx(Check)ListBox
  • margin alignment on owner-drawn menu item

Attachments (3)

owndrw.patch download (13.9 KB) - added by malcompl 5 years ago.
owndrw_msw.patch download (40.0 KB) - added by malcompl 5 years ago.
owndrw_os2.patch download (37.7 KB) - added by malcompl 5 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 6 years ago by vadz

  • Status changed from new to infoneeded_new

I still didn't have time to look at this in details so I might still be missing some problems but I'd be ready to apply this patch and deal with these problems later except for 2 things:

  1. It doesn't make sense to have __WXMSW__ and the old version of wxOwnerDrawn in wx/ownerdrw.h to me, this really duplicates too much. This class is only used in OS/2 code which is probably very similar to wxMSW, it would be great if it could be updated in the same way so that we could get rid of the old version.
  2. It seems that wxCheckListBoxItem should be changed more, e.g. to not store its m_nIndex -- it should derive from wxListBoxItem and reuse its GetIndex() IMO.

Would you be able to update the patch to do this? TIA!

comment:2 Changed 6 years ago by malcompl

  • Status changed from infoneeded_new to new
  1. I agreed with you, but I don't have the ability to do this on OS/2, but maybe we have on wx-dev some of OS/2 users, developers or even tester. If not, I can try install and build wx on this platform.
  2. Yes, I know. As I said in description, it is for the new patch. I prepared some more improvement and cleanup wxCheck/ListBox code, but I didn't include it here, because it leads to more confusion in the current code.

One more problem exists, adjusting menu labels on menu drawn item when icons are big fails, but probably this bug existed before this patch. Anyway, I prepared patch for it and I will send it after this one will be applyed.

comment:3 Changed 6 years ago by vadz

Alternative solution would be to move new version of this file to wx/msw/ownerdrw.h and keep the old one as wx/os2/ownerdrw.h. But modifying OS/2 sources, even blindly, would be better probably -- someone would usually fix compilation errors on OS/2 side but I don't think any one of two OS/2 developers has time to read your patch, understand how it works and reproduce its logic on OS/2 side.

comment:4 Changed 5 years ago by vadz

  • Status changed from new to infoneeded_new

Unfortunately I still don't think we can apply the patch in its current state. If modifying wxOS2 sources is not an option, could you please at least put the new version in wx/msw/ownerdraw.h as suggested above?

TIA!

comment:5 Changed 5 years ago by malcompl

  • Status changed from infoneeded_new to new

Patch updated. I modifed OS/2 source blindly, not tested, but it should be fine.
As I said in the past, the patch is modyfing other source (dependecy of wxOwnerDrawn class) only for logical and succesfull compilation, so some problems still exist on wxMSW, like bad apperance in wx(Check)ListBox and alignment on owner-drawn menu item, but I will fix it after this patch will be applyed.

Changed 5 years ago by malcompl

Changed 5 years ago by malcompl

Changed 5 years ago by malcompl

comment:6 Changed 5 years ago by vadz

  • Summary changed from wxOwnerDrawn refctore to wxOwnerDrawn refactoring

Thanks for the updated patch, I applied and test it and see that the wxCheckListBox appearance is indeed rather broken. I'd like to avoid having this (even in svn trunk) so instead of committing it there I'm going to commit it to a new svn "ownerdraw-refactor" branch. Could you please make the next patch (in another ticket please as this one starts getting long) against this branch? I'll then apply it there and then merge it back to the trunk when everything works.

TIA!

comment:7 Changed 5 years ago by VZ

  • Resolution set to fixed
  • Status changed from new to closed

(In [62041]) Refactor owner-drawing code.

Only keep common code in the base class and extract all menu/listbox-specific
stuff into derived classes.

This makes the code cleaner and more maintainable but introduces some problems
in wxCheckListBox appearance which will be fixed by the next patch.

Closes #10635.

comment:8 Changed 5 years ago by VZ

(In [63220]) Refactor owner-drawing code.

Only keep common code in the base class and extract all menu/listbox-specific
stuff into derived classes.

This makes the code cleaner and more maintainable but introduces some problems
in wxCheckListBox appearance which will be fixed by the next patch.

Closes #10635.

Note: See TracTickets for help on using tickets.