Opened 9 years ago

Last modified 3 years ago

#13210 new enhancement

wxAuiToolBar auto rotation

Reported by: R.U.10 Owned by:
Priority: normal Milestone: 3.2.0
Component: wxAui Version: stable-latest
Keywords: wxAuiToolBar auto rotation Cc:
Blocked By: Blocking: #10185, #10185
Patch: yes

Description

I extracted and enhanced my implementation reported in the ticket #10185

This implementation adds the ability of creating a toolbar with clockwise or counterclockwise rotation.
If the flag wxAUI_TB_ROTATE_AUTO is set, the toolbar rotates in the expected direction when it is docked on the frame border.

It implements:

  • some flags which allow to choose the orientation
  • the behaviour related to wxAUI_TBTOOL_TEXT_TOP flag
  • a centralized calculation of the position/rotation of the bitmap and the text of the toolbar tools
  • a new toolbar on the right of the sample dialog box

Attachments (1)

wxAuiToolBar_Auto_Rotation.patch download (37.5 KB) - added by R.U.10 9 years ago.
wxAuiToolBar auto rotation implementation

Download all attachments as: .zip

Change History (19)

comment:1 Changed 9 years ago by triton

  • Blocking

(In #10185) Can you re-upload the demo? The link is broken.

Can you also re-regenerate the patch updated to trunk?

comment:2 Changed 9 years ago by R.U.10

  • Blocking

(In #10185) I update the patch regularly so you can use it as is and compile the aui sample. It's more lasting than external links.

But new features are implemented in the trac #13210 for the handling of the toolbar. When the trac will be treated I will come back on the minimization behavior. Because IMHO the actual patch implements two complementary things but which should be treated separately.

comment:3 follow-up: Changed 9 years ago by vadz

  • Blocking

This patch seems more extensive but doesn't it duplicate the work of #11712 (see also r65061)? I.e. I think we already support toolbars which can be used in any orientation, don't we?

comment:4 in reply to: ↑ 3 ; follow-up: Changed 9 years ago by R.U.10

The work of #11712 implements the auto orientation of the toolbar according to the place where it is docked but the toolbar tools always stay horizontal.

My work is complementary because it implements the rotation of the toolbar tools depending on the orientation of the toolbar (text and icons are rotated of 90 or 270°).

I'm opened to discuss on the name to give at the functions and style macros to be more explicit. For example:
void SetOrientation(int a) { orientation = a; } could be renamed in
void SetRotation(int a) { rotation = a; }

wxAUI_TBTOOL_VERT_COUNTERCLOCKWISE could be
wxAUI_TBTOOL_VERTICAL_270

etc.

comment:5 in reply to: ↑ 4 ; follow-up: Changed 9 years ago by vadz

Replying to R.U.10:

The work of #11712 implements the auto orientation of the toolbar according to the place where it is docked but the toolbar tools always stay horizontal.

My work is complementary because it implements the rotation of the toolbar tools depending on the orientation of the toolbar (text and icons are rotated of 90 or 270°).

Ah, I see, thanks. Is it common to do this? I don't believe I've actually seen the tools rotate themselves like this in any of the applications I use.

I'm opened to discuss on the name to give at the functions and style macros to be more explicit. For example:
void SetOrientation(int a) { orientation = a; } could be renamed in
void SetRotation(int a) { rotation = a; }

wxAUI_TBTOOL_VERT_COUNTERCLOCKWISE could be
wxAUI_TBTOOL_VERTICAL_270

etc.

We definitely need to use some more clear names than "orientation" because this word is used for horizontal/vertical in many places in wx. Maybe "rotation" is indeed more clear. I'm still not sure to understand what do the constants above mean though, isn't the rotation determined by the orientation? I.e. if the toolbar is normally horizontal but is currently docked on the left, then the buttons should be rotated by 270 degrees, if it's docked on the right -- by 90 degrees (and I have no idea what should happen if it's docked at the bottom).

comment:6 in reply to: ↑ 5 Changed 9 years ago by R.U.10

Replying to vadz:

Replying to R.U.10:

implements the rotation of the toolbar tools (text and icons are rotated of 90 or 270°).

Is it common to do this? I don't believe I've actually seen the tools rotate themselves like this in any of the applications I use.

You are right but my initial idea was to release this behavior in prerequisite to the minimization of the AUI panes (see the blocked trac) even if I think now that it should be better to implement the rotation for the wxAuiNotebook tabs and to use a tab for the minimized pane state.
That said, I find that keeping the horizontal orientation of the toolbar tools is not aesthetic when the tools contain text, the toolbar can become very wide. So I propose this patch if it looks worthwhile for other people than me (you decide).

if the toolbar is normally horizontal but is currently docked on the left, then the buttons should be rotated by 270 degrees, if it's docked on the right -- by 90 degrees (and I have no idea what should happen if it's docked at the bottom).

If the toolbar is docked at the bottom, IMHO it is a little bit extravagant to have a 180° rotation so I did not implemented it, the tools are horizontal.
I don't know if it is desirable to rotate the text differently on the right and on the left. I think it could be disorienting. But perhaps some will prefer one direction over another, thats why I give the possibility to choose the both. But in the auto-rotation mode, the rotation is the same on the right and on the left (but I can modify it if you feel it is better to have a different rotation).

comment:7 follow-up: Changed 9 years ago by vadz

I didn't think about text but it does make sense to rotate it so I agree such functionality would be useful.

I think that rotating the text differently depending on whether the toolbar is docked on the left or right would be better, but this could be an option. So basically I see the following different modes of operation:

  • No rotation at all (default).
  • Automatic rotation as necessary (which would do what I suggest just because there doesn't seem any better way to name it).
  • Fixed rotation by 90 or 270 degrees (or clock- or counter-clockwise, as you prefer) to have the same orientation in both cases.

What do you think?

Also, what about the relationship with auto-orientation at the API level? I initially though it should be an error to specify the rotation for non-auto-orientable toolbar but now it looks like it could be useful for vertical toolbars too. Using any non-default rotation for the fixed horizontal toolbars should probably still be an error though?

comment:8 in reply to: ↑ 7 Changed 9 years ago by R.U.10

Replying to vadz:

What do you think?

That sounds good to me.

Using any non-default rotation for the fixed horizontal toolbars should probably still be an error though?

Not necessarily an error but a no effect option.

comment:9 Changed 9 years ago by R.U.10

Ok, I updated the patch with the recommendations, that seems good now.

I'm undecided about the name of the flag which defines "auto-orientation and auto-rotation text and icons":
wxAUI_TB_SMART_TEXT
wxAUI_TB_AUTO_TEXT
wxAUI_TB_SPIN_TEXT
...
I defined wxAUI_TB_SPIN_TEXT but I think it could be confusing with the spin control, what is your opinion?

Changed 9 years ago by R.U.10

wxAuiToolBar auto rotation implementation

comment:10 Changed 7 years ago by from_mars

  • Blocking

(In #10185) 1.
Thanks for your patch and sory for my english.

There are no such strings in wxWidgets 2.8.12 framemanager.h:

extern WXDLLIMPEXP_AUI const wxEventType wxEVT_AUI_PANE_RESTORE;
extern WXDLLIMPEXP_AUI const wxEventType wxEVT_AUI_RENDER;
extern WXDLLIMPEXP_AUI const wxEventType wxEVT_AUI_FIND_MANAGER;
.

They replaced by:

DEFINE_EVENT_TYPE(wxEVT_AUI_PANE_RESTORE)
DEFINE_EVENT_TYPE(wxEVT_AUI_RENDER)
DEFINE_EVENT_TYPE(wxEVT_AUI_FIND_MANAGER)
.

When i manuly added:

DEFINE_EVENT_TYPE(wxEVT_AUI_PANE_MINIMIZE)
DEFINE_EVENT_TYPE(wxEVT_AUI_PANE_MIN_RESTORE)

and everything compiled without errors.

3.
I compiled aui sample and ran it on Windows 7 64. Then I found it in Proccess Explorer, and there is such a thing - every time i minimizing panel with minimize button and then restoring it from created toolbar, Memory Working Set parameter in Proccess Explorer, grows up to about 24 Kbyte.
Could it be a memory leak?

comment:11 Changed 7 years ago by R.U.10

  • Blocking

(In #10185) Replying to from_mars:

Thank you for reporting that.

3.
Could it be a memory leak?

That was due to the creation of the toolbar during the pane minimization.
There was no memory leakage because the toolbar was properly deleted at the application closing, but it was not very clean.

I corrected all in the updated patch.

Regards

comment:12 Changed 7 years ago by vadz

  • Blocking

(In #10185) As usual, I'm going to struggle to review the patch of this size, especially in still unfamiliar to me wxAUI code. I wonder if splitting the text rotation patch could make it more manageable?

Any help/review from others (Jens Lody, mmarsan, ...) would be very welcome!

comment:13 Changed 7 years ago by jens

  • Blocking

(In #10185) Replying to vadz:

Any help/review from others (Jens Lody, mmarsan, ...) would be very welcome!

I will see if I find the time to look into it as soon as possible.

comment:14 Changed 7 years ago by from_mars

  • Blocking

(In #10185) For catching wxEVT_AUI_PANE_MIN_RESTORE event, i need use such code:

//wxAuiManager    m_mgr;
m_mgr.Bind(wxEVT_AUI_PANE_MIN_RESTORE, &wxContainerDBFrame::OnPaneRestore, this);

But for wxEVT_AUI_PANE_MINIMIZE, i can use such:

Bind(wxEVT_AUI_PANE_MINIMIZE, &wxContainerDBFrame::OnPaneMinimize, this);

Does it normal?

comment:15 Changed 7 years ago by vadz

  • Blocking

(In #10185) No changes, just resetting the (probably accidentally) changed status.

comment:16 Changed 7 years ago by R.U.10

  • Blocking

(In #10185) Replying to from_mars:

It's due to the processing of the event MIN_RESTORE which is not redirected to the manager's frame like the others. See the commented line in the file auibar.cpp l.2773:

                    //manager->ProcessMgrEvent(e);
                    manager->ProcessEvent(e);

Uncommenting this line (and commenting the following) will probably resolve the issue but I don't know why this line is commented (is there unexpected border effects?).

ninjanl is the author of this part of code, perhaps he can say more than I?

comment:17 Changed 7 years ago by vadz

  • Blocking
  • Milestone set to 3.2

This should be dealt with as part of AUI rewrite/refactoring.

comment:18 Changed 3 years ago by hackish

  • Blocking
Note: See TracTickets for help on using tickets.