Prevent file tabs moving to the bottom row when a tab on top row was activated?

176 views
Skip to first unread message

Va no

unread,
Jun 21, 2020, 6:26:59 PM6/21/20
to scite-interest

Hello.

When tabbar.multiline is enabled and there are more then one row of file tabs (buffers), clicking on a tab on top row will move the entire top row to the bottom. This makes it a nightmare to track where is what. Is there a setting disable this behaviour?


Thank you.

Lorenzo Donati

unread,
Jun 22, 2020, 4:51:11 AM6/22/20
to scite-i...@googlegroups.com
On 21/06/2020 10:13, Va no wrote:
>
>
> Hello.
>
> When *tabbar.multiline* is enabled and there are more then one row of file
> tabs (buffers), clicking on a tab on top row will move the entire top row
> to the bottom. This makes it a nightmare to track where is what. Is there a
> setting disable this behaviour?
>
>
> Thank you.
>

I'd like to chime in on this because I was about to post a question like
yours one of these days. It's fairly annoying indeed (I often work with
several tabs open which I need to switch frequently, so the single line
tabbar won't cut it).

I have a hunch, though, this is has to do with the specific GUI widget
used (btw, I use Windows). I hope there is a flag on that widget that
can be used to stop the tabs lines from moving. Otherwise a new widget
must to be used or, worse, implemented.

Cheers!

-- Lorenzo

Neil Hodgson

unread,
Jun 22, 2020, 5:33:26 PM6/22/20
to scite-interest
Lorenzo:

> I have a hunch, though, this is has to do with the specific GUI widget used (btw, I use Windows).
> I hope there is a flag on that widget that can be used to stop the tabs lines from moving.

If anyone finds a way to implement this on the the system's SysTabControl32 widget then it can be incorporated into SciTE.

> Otherwise a new widget must to be used or, worse, implemented.

That’s a lot of work. On macOS, SciTE uses a custom widget for the tab bar and it doesn’t have all the functions of the Win32 equivalent.

Neil

Lorenzo Donati

unread,
Jun 23, 2020, 6:10:31 AM6/23/20
to scite-i...@googlegroups.com
Disclaimer: I've no experience in Windows API GUI programming (and
little in GUI programming in general), so I may well missing something
obvious.

I just spent half an hour browsing MS online docs, especially this:

https://docs.microsoft.com/en-us/windows/win32/controls/tab-control-styles

And it appears we are not lucky for finding an easy solution. There is
no mention of any flags/styles that can be set to alter multiline tab
bar behaviour.

The only style with obscure meaning (to me, at least) is this:

TCS_SCROLLOPPOSITE
"Unneeded tabs scroll to the opposite side of the control when a tab is
selected."

Which maybe could do what we want if used in a TCS_MULTILINE syled tab
bar, but it's just a wild guess.


Anyway, probably one could handle the messages sent to the control and
add a custom painting routing, but that's a lot of work for little gain.

That's almost as complicated as implementing a new control, again, for
little gain.

The only easy solution would be finding a lightweight open source widget
library with a better tab bar control. Most probably not worth the hassle.

Sadly, I fear we are stuck with that multiline tabbar annoying behaviour.


Cheers!

-- Lorenzo






Message has been deleted

Va no

unread,
Jun 23, 2020, 7:41:07 AM6/23/20
to scite-interest
Well, I don't know how, but Notepad++ is based on Scintilla and they managed to fix this issue...
https://github.com/notepad-plus-plus/notepad-plus-plus

Lorenzo Donati

unread,
Jun 23, 2020, 9:16:32 AM6/23/20
to scite-i...@googlegroups.com
On 23/06/2020 12:44, Va no wrote:
> I'm totally out of my league here, but would it be too complicated
> use multiple single line tabbars instead?
>

In theory that would be possible, but it would probably almost amount to
writing an entire new widget.

In fact you would need to handle the interactions between the tabbars.
For example: if there are two tabs on the first and one on the second
tabbar and you closed the second tab on the first bar, you would have to
merge the tabbars. Otherwise you would have two tabbars with one tab each.

Another example: if you had one tabbar with two tabs and wanted to add
one tab, to avoid the default behavior of single line tabbars, you would
detect that the 3rd tab would not fit in the bar and create one, while
inhibiting the default behavior.

Probably it is doable, but there is an incredible amount of work to make
it smooth. For what gain? Moreover, the code wouldn't be clean at all.

The whole point of having already made widgets is to avoid all this
hassle. Since SciTE is not a big application with a refined GUI, but it
is just a nice application built around Scintilla, I doubt this is feasible.

Moreover tab bars are not meant to be used the way you implied. I never
saw an application with two independent tab bars in the same "visual
space": it is possible that the graphic effect would be terrible.

As I said, probably it would just be cleaner to intercept messages to a
multiline tab bar, assuming it is even possible with that widget to have
a focused tab not being brought in the first tab line.

At the end of the day, the development effort would be too much for too
little gain, IMO.

> On Tuesday, June 23, 2020 at 10:10:31 AM UTC, Lorenzo Donati wrote:
>>
>> On 22/06/2020 23:33, 'Neil Hodgson' via scite-interest wrote:
>>> Lorenzo:
>>>

[snip]

Dejan Budimir

unread,
Jun 29, 2020, 5:03:11 AM6/29/20
to scite-interest
Hi.

This is annoying. I haven't used the tabbar in a while. Now I remember why. Also because it flickers. ( And because it sometimes triggers a CTRL+A on the editor, which feels like a race condition; althoguh this happens predictably if you right click on a tab bar, is this a feature? ). It is very unpleasant to use!

But I think there is a way.

The trick is to use LBS_BUTTONS (but only in multline mode). The tabs will look a little different (i feel they're nicer now), but the control is ugly anyhow. There's that TCS_FLATBUTTONS style to play with in case that makes it better.

Tests have yielded the following:

TCS_SINGLELINE | TCS_TABS : OK, works as expected.
TCS_SINGLELINE | TCS_BUTTONS : rearranges the tabs in strange ways.
TCS_MULTILINE | TCS_TABS : is the cause for the issue here and may wreak havoc on the unsuspecting mind.
TCS_MULTILINE | TCS_BUTTONS : OK, works as expected.

So we should use TCS_TABS with TCS_SINGLELINE and TCS_BUTTONS with TCS_MULTILINE. If the visual difference is an issue, we could still always give TCS_OWNERDRAWFIXED a try.

There's a tiny issue here (I'm still searching for the docs on this). The tab control seems to no longer send TCN_SELCHANGE notifications but sends NM_CLICK instead. The required changes though are minimal and work fine in my quick tests.

Also note that LBS_MULTILINE can be toggled during the control's lifetime, but LBS_BUTTONS can only be set during creation. But this shouldn't be a problem, i.e. to create a brand new control at property change (given how the tab bar is a flickering mess already). Morover, as it is now, changing the tabbar.multiline property seems to require an app restart to take effect (obviously I believe we can do better here).

Patch is coming up over at SF.

Lorenzo Donati

unread,
Jun 29, 2020, 6:06:46 AM6/29/20
to scite-i...@googlegroups.com
On 29/06/2020 11:03, Dejan Budimir wrote:
> Hi.
>
> This is annoying. I haven't used the tabbar in a while. Now I
> remember why. Also because it flickers. ( And because it sometimes
> triggers a CTRL+A on the editor, which feels like a race condition;
> althoguh this happens predictably if you right click on a tab bar,
> is this a feature? ). It is very unpleasant to use!
>

I haven't used the tabbar for ages, but I recently switched to v.4.3.3
(for various reasons I was stuck with v.3.3.x) and needed to work with
lots of files. I gave tabbar a shot again, and it wasn't SO ugly.

That is, I never noticed that CTRL+A issue and it doesn't flicker (I use
Windows 7 - yes old stuff, but much less talky-talky to MS than Win10 :-)

For me the really annoying thing is that row-swapping thing. I mitigated
it by defining Ctrl+TAB as a shortcut to switch buffers and using the
property

buffers.zorder.switching=1

So that I can easily switch between one buffer and the one I previously
selected.

It's not perfect, but manageable. Still, looking at the multiline tabbar
while switching buffers in fast succession gives me an headache!

Whoever designed that control probably never UX-tested it seriously.
That's also why it seldom appears in Windows GUI. Most of the time
tabbars are single line. The only ones I remember which are multiline
are internet Settings and mouse settings in control panel.


> But I think there is a way.
>
> The trick is to use LBS_BUTTONS (but only in multline mode). The
> tabs will look a little different (i feel they're nicer now), but
> the control is ugly anyhow. There's that TCS_FLATBUTTONS style to
> play with in case that makes it better.
>
>
> Tests have yielded the following:
>
> TCS_SINGLELINE | TCS_TABS : OK, works as expected. TCS_SINGLELINE
> | TCS_BUTTONS : rearranges the tabs in strange ways. TCS_MULTILINE
> | TCS_TABS : is the cause for the issue here and may wreak havoc
> on the unsuspecting mind. TCS_MULTILINE | TCS_BUTTONS : OK, works
> as expected.
>

Nice find!

> So we should use TCS_TABS with TCS_SINGLELINE and TCS_BUTTONS with
> TCS_MULTILINE. If the visual difference is an issue, we could still
> always give TCS_OWNERDRAWFIXED a try.
>

If the graphic result is not totally ugly, I wouldn't mind if its
appearance is a bit odd.

I'm always against adding more code bloat if it is just for some /minor/
aesthetic reason. OTOH, if using TCS_OWNERDRAWFIXED can fix the issue
elegantly with a few code lines that's OK.

> There's a tiny issue here (I'm still searching for the docs on
> this). The tab control seems to no longer send TCN_SELCHANGE
> notifications but sends NM_CLICK instead. The required changes though
> are minimal and work fine in my quick tests.
>

I guess that behavior is reasonable: as I get it from the docs,
TCS_BUTTONS style makes the tab bar just a row of buttons, not a "real"
tabbar. I mean, I guess the buttons are completely independent and don't
"know" of each other, so they use a more basic notification.

> Also note that LBS_MULTILINE can be toggled during the control's
> lifetime, but LBS_BUTTONS can only be set during creation. But this
> shouldn't be a problem, i.e. to create a brand new control at
> property change (given how the tab bar is a flickering mess
> already). Morover, as it is now, changing the tabbar.multiline
> property seems to require an app restart to take effect (obviously I
> believe we can do better here).
>

I don't think it is a big issue. I would prefer to restart SciTE instead
of having a little more convenience in a rare use case at the cost of
code bloat and complications.

If more complex code is to be added, I would prefer a ML tabbar having a
limit on the number of lines, above which the lines could be scrolled.
That is, the way a multiline tab works in some browsers like Firefox.
Very handy: over (say) 3 tab rows, you can use mouse wheel (or arrows
buttons) to scroll the lines. Extremely efficient user experience when
continuously switching between tabs. Coupled with the ability to drag
and rearrange tabs it's a huge productivity booster, IMO.

Probably all this could be solved by using TCS_BUTTONS and
TCS_OWNERDRAWFIXED and more coding, but it seems quite a lot of work.
Anyway I'm just guessing. I'm no GUI programming guru and just had some
limited experience with Java GUI Swing library more than a decade ago!

> Patch is coming up over at SF.
>
Care to share a link?


-- Lorenzo

Dejan Budimir

unread,
Jun 29, 2020, 8:22:50 AM6/29/20
to scite-interest
Hi Lorenzo.

> Windows 7 - yes old stuff

I'm still stuck on winXP, so no judgement from me ;).

> I mitigated it ...

It's always good to try a workaround first, but I feel you were much too patient here!

> Whoever designed that control probably never UX-tested it seriously.

True.

> If the graphic result is not totally ugly ...

Actually, I think it's more beautiful. I might use it more often now myself. Just a grid of buttons. I've posted a screencap (see below) since this is going to take a little longer than expected (ditto).

> I guess that behavior is reasonable: as I get it from the docs, TCS_BUTTONS style makes the tab bar just a row of buttons ...

Thankfully the individual buttons do act as a group, hence like a tab bar, still. My expectation was that, if it's in _essence_ a tabbar control, then it should adhere to the same user-interface regardless of its _accidental_ styles. I suspect the design decision here was to keep it as light-weight as possible: they simply plugged in button controls and didn't subclass their behavior to conform to the tabbar contract.

> I would prefer to restart SciTE instead of having a little more convenience

True. Unless ... with 30+ files open, I'm not so sure (session save/restore helps, alas there's the file-loading overhead ... and what if I really want to quickly toggle all the tabs into view and out again?). To misquote JFK, ask not what you can do for SciTE etc.

In any case, I know that this is not a requirement of Windows, to have to restart an app in order to change chrome. Now I'm hoping that it's not a requirement of SciTE's cross-platform nature (see below).

> I'm always against adding more code bloat ...

I'm trying hard not to change too much here.

> Care to share a link?

It's not ready yet, but there is a screencap here <https://sourceforge.net/p/scintilla/feature-requests/1366/>.

Dejan Budimir

unread,
Jun 29, 2020, 10:11:56 AM6/29/20
to scite-interest
So, pffff. This is me having the air let out of me. I just saw what I was up against. Each and every time you click a tab, it empties and repopulates the tab bar! This is why the tabs are dancing even in single-line mode (e.g. when you've scrolled to the right and click a tab).

Dejan Budimir

unread,
Jun 29, 2020, 11:34:21 AM6/29/20
to scite-interest
Correction. I've claimed that the control needed to be re-created to set the TCS_BUTTONS flag, thankfully that was an error on my part. It made things *a lot* easier. See here for reference: <https://docs.microsoft.com/ru-ru/windows/win32/controls/tab-control-styles>

The patch is here <https://sourceforge.net/p/scintilla/feature-requests/1366/#6bcd>. Nothing special.

Neil Hodgson

unread,
Jul 1, 2020, 9:08:00 AM7/1/20
to scite-i...@googlegroups.com
Dejan Budimir:

> So, pffff. This is me having the air let out of me. I just saw what I was up against. Each and every time you click a tab, it empties and repopulates the tab bar! This is why the tabs are dancing even in single-line mode (e.g. when you've scrolled to the right and click a tab).

The tab bar contents are likely equivalent to just a list of strings and the clear/repopulate could be avoided unless the new list is different to the current list. There is shared code here with the buffers menu but the menu is more complex as it attaches command identifiers to menu items. The tab bar is just position-based.

Neil

Neil Hodgson

unread,
Jul 19, 2020, 6:59:56 PM7/19/20
to scite-interest
Dejan Budimir:

> ... Each and every time you click a tab, it empties and repopulates the tab bar! This is why the tabs are dancing even in single-line mode (e.g. when you've scrolled to the right and click a tab).

Repopulating the tab bar can be optimized with this patch which keeps a copy of the tab text and only updates changed, added, or removed tabs.

It doesn’t really fix multiline tabs which still do a lot of redrawing. The reason the tab control moves the active tab to the bottom is that its folder-tab-open-at-bottom tries to make the tab appear attached to the document. The button mode isn’t trying to maintain this appearance so doesn’t move.

Neil
UpdateTabs.patch
Message has been deleted

Dejan Budimir

unread,
Oct 10, 2020, 3:26:40 PM10/10/20
to scite-interest
Sorry for the long delays.

Repopulating the tab bar can be optimized with this patch which keeps a copy of the tab text and only updates changed, added, or removed tabs. 
 
The attached patch is similar but avoids duplicating the TabBar's memory (which is a minor issue really). It does so at the cost of calling into the message loop and copying strings around (essentially a minor issue too). Also it doesn't change the class interface but at the cost of changing the semantics of TabInsert (which now synchronizes tabs to buffers like your UpdateTabs) and RemoveAllTabs (which is a non-op now). The new semantics actually reflect how the methods are used: i.e. to synchronize two stacks under the assumption that TabBar operations are cheap.

It also fixes an additional culprit in SizeSubWindows which significantly reduces flickering. Maybe you can use that part.

The reason the tab control moves the active tab to the bottom is that its folder-tab-open-at-bottom tries to make the tab appear attached to the document. The button mode isn’t trying to maintain this appearance so doesn’t move.

True. These are two different metaphors. The ugliness lies with the the one mode not being a drop-in replacement for the other, otherwise the OP's request would have been easy to address.

Seeing how many functions (indirectly) call into RemoveAllTabs and the subsequent (often four-fold) TabInsert ...

diagram.png

the patch can be significantly optimised with hashing.

Because TabInsert() is called so often (four times for every open tab, see attached trace log) the comparison between buffer titles and tab should preferably not be a simple string comparison. Because in the vast majority of cases all the strings (minus one) will be identical, comparison will never break off and every byte will have to be visited.

A simple hashing algorithm for windows paths could store the path in the short 8.3 DOS format. A more sophisticated method should exploit the specific format of file paths and thereby hopefully fit inside 4 bytes, so we could directly use the TC_ITEM's user param field without having to allocate more on the heap.
multibar.patch
trace-log.txt

Neil Hodgson

unread,
Oct 10, 2020, 5:44:23 PM10/10/20
to scite-interest
Dejan Budimir:

> The attached patch is similar but avoids duplicating the TabBar's memory (which is a minor issue really). It does so at the cost of calling into the message loop and copying strings around (essentially a minor issue too).

I originally wrote a version that called TabCtrl_GetItem but that imposes a fixed size buffer - MAX_PATH in your patch. While long paths do not work in all parts of SciTE yet, adding more limitations will make long path support more difficult. There appears to be no way to determine the size of a tab item’s text to allocate the amount needed.

The patch is using its own logic for determining the correspondence between buffers and tab text but does not appear to take into account read-only ‘|’ or dirty ‘*’ indicators. File names that include ‘&’ may also be an issue as they are modified by EscapeFilePathsForMenu.

> the patch can be significantly optimised with hashing.
>
> Because TabInsert() is called so often (four times for every open tab, see attached trace log) the comparison between buffer titles and tab should preferably not be a simple string comparison. Because in the vast majority of cases all the strings (minus one) will be identical, comparison will never break off and every byte will have to be visited.

The time cost of GUI redrawing is huge compared to computation cost. Unless the comparison appears significant in a profile, it likely isn’t worth pursuing.

Neil

Dejan Budimir

unread,
Oct 11, 2020, 2:49:51 AM10/11/20
to scite-interest
 
I originally wrote a version that called TabCtrl_GetItem but ...

My second look at your patch certainly made me appreciate it more than the Win32 approach. I just find it hard to get away from how Windows wants us to use it.
 
There appears to be no way to determine the size of a tab item’s text to allocate the amount needed.
 
That's easy. We could store the string length into TC_ITEM's lParam when we set it.

The patch is using its own logic for determining the correspondence between buffers and tab text but does not appear to take into account read-only ‘|’ or dirty ‘*’ indicators. File names that include ‘&’ may also be an issue as they are modified by EscapeFilePathsForMenu.

Right, I hadn't made an inventory of all the possible annotations yet. But the comparison here is not the final form, since the tab titles are not full paths anyway and hence aren't sufficient to determine identity with buffers. The move to hashes was much on my mind.

The hash collisions would not be resolvable by comparing only the filename parts though. So I considered simply storing the Buffer objects address in TC_ITEM::lParam, which would be the most efficient solution, but I didn't pursue that thought further because I wasn't yet sure about the impact of making a Buffer's address be guaranteed to remain constant during its lifetime, given the use of dynamic memory (std::vector). Also, although a minor issue, on 64 bit machines we would have to define a default structure for the TC_ITEM::lParam and hence be allocating on the heap, which I was trying to avoid but I now accept the futility of that. I would absolutely love to be able to simply store, inside the freely available TC_ITEM user param space, the address of its corresponding buffer. Possibly we could derive our own allocator for the vector so that it triggers an update of each TC_ITEM's lParam whenever the memory layout changes?

The time cost of GUI redrawing is huge compared to computation cost. Unless the comparison appears significant in a profile, it likely isn’t worth pursuing.

You are perfectly right. But I'd say that we should pursue it where it is easy to pursue, regardless of its relative worth. And I think I'll be looking into the approach laid out above, if you find no fault with it, so that we get an integer compare instead of the (long path) string compare.

As for the priority on combating unnecessary redrawing, please comment on the small change to SizeSubWindows. Especially as for the use of statics here. The problem here was moving the control with SetWindowPos without SWP_NOREDRAW. 

Dejan Budimir

unread,
Oct 11, 2020, 3:51:09 AM10/11/20
to scite-interest
Possibly we could derive our own allocator for the vector so that it triggers an update of each TC_ITEM's lParam whenever the memory layout changes?

I was too optimistic there. The slightly better approach would be to replace BufferList::buffers with a custom object that exposes the same interface as a std::vector. It would have to check that the addresses hadn't changed (by looking the first element's address after every push_back, say; since the contents are contiguous in memory, checking the address of a single element suffices). The best approach would be to not even touch the interface but du this checking in TabInsert instead.

Dejan Budimir

unread,
Oct 11, 2020, 5:01:25 AM10/11/20
to scite-interest
 It would have to check that the addresses hadn't changed ...

Actually, it would appear that capacity of BufferList::buffers is invariant. I'll pretend that this may be altered in the future and add code to handle this contingency regardless. It's always easier to delete a feature than to add it.

Dejan Budimir

unread,
Oct 11, 2020, 5:24:25 AM10/11/20
to scite-interest
Say, Neil, what is the purpose of:

    GUI::gui_string titleCopy(title, title + wcslen(title) + 1);
    tie.pszText = &titleCopy[0];

in TabInsert()? gui_string is just a typedef of std::wstring. And TabBar copies that string into an internal buffer, rather than taking over the string's memory, say. Even so, the string is created on the stack so the memory would be deallocated anyway.

This innocent looking line visits every byte of the string twice. If it serves no purpose, let's get rid of it.

Dejan Budimir

unread,
Oct 11, 2020, 6:56:10 AM10/11/20
to scite-interest
So this is what I'm thinking of so far. RemoveAllTabs is a no-op. At every TabInsert:
  • the TB (TabBar) is synced to the BL (BufferList) so that their item counts match, and
  • so that every T (tab) knows its B (Buffer) object via the T's lParam.
There is lots of skipping possible here, optimising the whole process.

(This way it's easy to change the TB to be owner-drawn. I find it helps me think about it, if I assume owner-draw, because it nicely splits the responsibilities. TabInsert should care about growing and shrinking the TB, and maintaining T-B associations. A T, knowing its B, should know how to make its own title.)

Now we need to figure out when to avoid setting a T's title. Preferably without comparing strings. A B's state is changing (even in-between calls to TabInsert), to reflect e.g. dirty flags. Its associate T's title is changing only to reflect some such changes, let's call those BS (BufferState). So what we want is to know (at T-drawing time, in the case of owner-draw) whether BS is dirty, so that we can derive a new title from the current BS.

Is this making sense? Sometimes I miss the forest for the trees. Often for the shrubbery as well.

Neil Hodgson

unread,
Oct 11, 2020, 5:20:42 PM10/11/20
to scite-i...@googlegroups.com
Dejan Budimir:

> Say, Neil, what is the purpose of:
>
> GUI::gui_string titleCopy(title, title + wcslen(title) + 1);
> tie.pszText = &titleCopy[0];

Const correctness. const_cast should be strongly avoided.

> This innocent looking line visits every byte of the string twice. If it serves no purpose, let's get rid of it.

Not unless it can be shown to be a problem.

> As for the priority on combating unnecessary redrawing, please comment on the small change to SizeSubWindows. Especially as for the use of statics here. The problem here was moving the control with SetWindowPos without SWP_NOREDRAW.

Function-local statics are bad as they are hidden global state. The current tab bar position is already known to Windows so whether the SetPosition is needed could be checked against that. Its possible that Windows already optimizes a no-op SetPosition.

The tab bar is a band and should be repositioned later by the band layout loop. The purpose of that SetWindowPos is so that TabCtrl_AdjustRect takes the new width into account for calculating the height needed. This may not be necessary if TabCtrl_AdjustRect is a pure function that doesn’t need the actual tab bar to have some width. This code was originally more complex to deal with old versions of Windows and some of the complexity has remained.

> The hash collisions would not be resolvable by comparing only the filename parts though. So I considered simply storing the Buffer objects address in TC_ITEM::lParam, which would be the most efficient solution, but I didn't pursue that thought further because I wasn't yet sure about the impact of making a Buffer's address be guaranteed to remain constant during its lifetime, given the use of dynamic memory (std::vector).

Having secondary references to objects can lead to mismatches. A single source of truth is much less likely to go wrong. For me, modelling the tab bar contents as a simple vector of strings is a great simplification that should be robust against future needs and is likely to work cross-platform.

Neil

Dejan Budimir

unread,
Oct 11, 2020, 11:03:24 PM10/11/20
to scite-interest
( Don't let me waste your time. Nothing in this post is urgent. A real patch is forthcoming. )

Aye to the avoidance of hidden state. But nay to const correctness in this case. Windows takes a LPWSTR instead of an LPCSTR here only because the field is dual-purpose. It will never touch the string, so there's no worries. The const_cast is there for reasons such as this.
 
Not unless it can be shown to be a problem.  

It's an expensive non-op, that's all. But there's worse.
 
The current tab bar position is already known to Windows so whether the SetPosition is needed could be checked against that.

    GUI::Rectangle(rcClient.left, rcClient.top + visHeightTools, rcClient.right, rcClient.top + heightTab + visHeightTools)

The only variables here are rcClient.right and visHeightTools. It will always change/redraw the multi-row TBs. Then change/redraw again with DeferPos.

Its possible that Windows already optimizes a no-op SetPosition.

I don't think it does, but even so, the bar is not moved in-place andthere was significant improvement.
 
This code was originally more complex to deal with old versions of Windows and some of the complexity has remained.

I understand. Times are changing though. Wasted clock cycles are speeding up the heat death of the universe.
 
A single source of truth is much less likely to go wrong. For me, modelling the tab bar contents as a simple vector of strings is a great simplification that should be robust against future needs and is likely to work cross-platform.
 
 It is objectively the better solution in this case. My approach (see patch) worked nicely (see logs) but finally it fails when it comes to shifting tabs around. There simply is no way to do it, sanity-wise. But the path forward is now clear. I'll move away from TC_ITEMs in favor of a vector (gladly not of strings though, using much of what is already in this patch).
log.txt
notquite.patch

Neil Hodgson

unread,
Oct 12, 2020, 6:01:21 PM10/12/20
to scite-i...@googlegroups.com
Dejan Budimir:

> It is objectively the better solution in this case. My approach (see patch) worked nicely (see logs) but finally it fails when it comes to shifting tabs around. There simply is no way to do it, sanity-wise.

This was informed by SwiftUI and some of the more functional JavaScript UI frameworks. It is difficult to code every possible state mutation operation so it can be better to write a single function that produces a logical state then apply that logical state to the concrete UI elements.

Functional UI frameworks have generic differencing engines that can discover a minimal (or near minimal) set of updates that synchronizes the concrete UI. SciTEWin::UpdateTabs isn’t generic but it does behave reasonably for most changes I have looked at.

Neil

Dejan Budimir

unread,
Oct 13, 2020, 6:12:48 AM10/13/20
to scite-interest

This was informed by SwiftUI and some of the more functional JavaScript UI frameworks.

Yes, React came to my mind as well when I realized what was involved in keeping all the optimisations confined to only the SciTEWinBar.cxx. I essentially reinvented the wheel and only when I posted it did I see that you had already solved this. I never thought that you'd take out the time from your schedule to do this. Thanks for bothering that much about what is perhaps a minor feature like the TabBar.

( My XP Box died this summer and I lost all motivation to continue with it, so I simply forgot about it. I'm on Windows 10 now, finally. )

The patch here is the final version. It works extremely well for me.

The patch includes the proposed changes to one other file than win32/SciTEWinBar.cxx, but it isn't necessary yet. The changes to SciteWinBar.cxx will work without any changes to src/SciTEBase.h (avoiding longish recompilations). But see the comments for BufferTitleState::numberOfTimesLoaded. However, the states vector is file global to the *.cxx and you may want to put it into a class declaration, so recompilatoin will still be necessary.

In the log you'll see how it only updates a tab's title when the relevant buffer state is changed. The format of each line is "TabInsert[0]" (to indicate that we're doing everything in TabInsert at index 0) then, in brackets, a sequence of chars for every tab. The chars indicate what reason there was for updating its title, or '-' if updating was skipped. The logs are not as informative without looking at the code though.

I believe I've identified the tab-title-relevant buffer states. But there may be stuff that happens via Lua scripting which I didn't really take the time to think about at all. It would surprise me if there were such side-effects though. As long as TabInsert (which is only called by SetBuffersMenu as far as I could tell, see diagram above) is being called whenever the buffers change for whatever reason, we should be fine.

Instead of a vector of tab titles, we now use a vector of BufferTitleState (because BufferState was already taken). A BufferTitleState's size should be some measly 16 bytes or so (sizeof 2 pointers + 1 int + 3 bools + implicit alignment padding), and its methods are guaranteed to finish in constant time (specifically, there are no longer any string comparisons; considering that UNC strings could be anywhere up to 30k wide chars long, that's desirable).

You'll notice that I've identified a common algorithm and put it into a template named "i_until" (for want of better name). It was surprising (though obvious once really thought about it) that it underlies both growing and shrinking the TabBar and states vector but also for swapping of vector elements during tab-shifting. Feel free to inline this instead, by writing out the loops by hand. For some reason I find it easier to read with i_until, but future readers of the code may have to suffer a double take.  I've also used swapping to shift tabs instead of the copying approach utilized in e.g. BufferList::ShiftTo. (Buffer objects look swappablet to me, so maybe we should use swaps there too.)

A further slight optimization is possible if it is guaranteed that TabInsert is called after every single tab insert/delete, which I believe to be the case. In other words, whenever the TabBar is synced to the buffers, the difference in sizes will be 1 at most. If so then we can get rid of a few loops.

All in all, I'm happy with it so far. I'd start focussing on getting the TBS_BUTTONS style to work in the future.

( Sorry for git-diff's way of deleting and re-adding the same lines  in order to minimize the patch. I tried several arguments, in particular "--anchored=" but was unable to make the patch more readable. )
trace-log.txt
mutlibar-2.patch

Dejan Budimir

unread,
Oct 28, 2020, 6:34:54 AM10/28/20
to scite-interest
So here it is, finally. This makes TCS_BUTTONS work.

This is the actual fix for OP's issue. My previous patch, obviously, is orthogonal to this. I would love to see both making it into production.
0.patch

Dejan Budimir

unread,
Oct 28, 2020, 6:46:00 AM10/28/20
to scite-interest
I would love to see both making it into production.

Both patches need a good stress-test, of course.

The previous patch needs cleaning up and adjustment to coding-style. Also Neil needs to decide where to put the new class and the functions; the title-generating function in particular needs to be put somewhere nice, so that the original (now duplicate) code can be eliminated for PLAT_WIN.

I'm actually happy now! My general annoyance with this was definitely the drive I needed to stay with it. Sorry for having been  ... difficult. ;)

van...@gmail.com

unread,
Mar 14, 2021, 8:57:08 AM3/14/21
to scite-interest
Would you update it on feature request website too?
https://sourceforge.net/p/scintilla/feature-requests/1366/

jenny...@163.com

unread,
Mar 16, 2021, 12:22:17 AM3/16/21
to scite-interest
Hi, neil

    in my program, caret still active when editor pane not active.

    There are multiple editor windows in my program.

    When the editor window is not active, for example you click in the console pane, the text caret is still visible and flashing in the editor - meant to indicate that it is ready to have text typed there, however it is not, keystrokes are ignored until you click in the editor. 

    The simple editor works correctly. 





Neil Hodgson

unread,
Mar 25, 2021, 5:25:22 PM3/25/21
to scite-interest
jenny.venus:

    in my program, caret still active when editor pane not active.

    There are multiple editor windows in my program.

    When the editor window is not active, for example you click in the console pane, the text caret is still visible and flashing in the editor - meant to indicate that it is ready to have text typed there, however it is not, keystrokes are ignored until you click in the editor. 

   Scintilla acts on focus notifications from the system. When it receives a kill focus event, it will hide the caret. However, if the new focus window is a child of that Scintilla instance (either the autocompletion or a calltip) then it will not hide the caret.

   The application may also override the current focus status by calling SCI_FOCUS which can be useful when the application wants an unusual feature such as typing into two scintilla instances at once.

   Neil

jenny...@163.com

unread,
Mar 26, 2021, 1:44:58 AM3/26/21
to scite-interest
Thank you, neil

I didn't find a specific and reliable approach from Scintilla, but after a careful investigation, I found that my previous code has logical bug and wrong.

My previous version logic is as follows

1. Create a window
2. Create all Pane and and all child-window loaded in PANE, such as Tree, Hexeditor, etc., and hide some PANE
            (At this point, some PANE loaded Scintilla, but this PANE does not have focus, and the main window has just created at this time)
3. Then the things previously described appeared
           (When this happens, you need to use controls within the PANE with SCINTILLA to use the mouse point)

The current version logic is as follows

  1. Create a window
  2. Create a part you need to display the PANE, and create the child window loaded in these Panes.
  3. Other PANE and child windows, create when the first time you need to display
  4. BUG solves


--
You received this message because you are subscribed to the Google Groups "scite-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scite-interes...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/scite-interest/40F66457-1F4D-402D-873A-96D86C248C83%40me.com.

Neil Hodgson

unread,
Apr 7, 2021, 5:12:18 AM4/7/21
to scite-i...@googlegroups.com
Dejan Budimir:

> So here it is, finally. This makes TCS_BUTTONS work.

This patch doesn’t seem to handle a click without drag: the moving cursor and red arrows appear after mouse up. It may need to reset more state on mouse up.

Neil

Neil Hodgson

unread,
Apr 13, 2021, 8:11:39 PM4/13/21
to scite-i...@googlegroups.com
Me:

> Repopulating the tab bar can be optimized with this patch which keeps a copy of the tab text and only updates changed, added, or removed tabs.

Something similar to this has been committed.
https://sourceforge.net/p/scintilla/scite/ci/bd88246718179220049336a7ef366f87152c946b/

Its a bit more refined than the previous patch. Tab removal deletes the tab being removed instead of deleting the last then updating all the titles. A similar change is made for tab insertion but that always happens at the end so its not an improvement although it would be if tabs were inserted next to the current tab.

The algorithm: [delete excess items; insert additional items; update changed items] could be turned into a template and applied over menus as well but there is no real need as the menus are normally hidden so don’t flash. Profiling shows UpdateTabs uses almost no CPU but repainting the tabs, which is deferred, does.

Neil

Reply all
Reply to author
Forward
0 new messages