ATN Vincenct Snijders, Dmitry Boyarintsev, chronos... Mantis #18820, #18898, #18961...

The ListView component is in a bad state, and a review of the current item handling is required, before proper solutions can be found. Problems arise with OwnerData and sorting, and at least OwnerData should be made work properly.

A general decision should be made, about the role of the ListItems class/field. Currently this member is used as a shadow of the list items, as stored in the widget, with the same number and arrangement of the list items in the control and widget. This list is unusable in OwnerData mode, where AllocBy holds the item count, and a callback is used to retrieve the data of the current item.

The communication with the widget is based on the index of the currently selected item. This can work in both modes, when properly handled in the message handlers. A problem arises when the list is sorted, because then the items in both the control and widget have to be sorted in parallel. It would be nice to make the interface work *without* the need of sorting both lists together.
The Delphi version has no such problem, because the items are stored *only* in the widget, not in the control. This may be inapplicable to non-Windows widgetsets, so that this essential part of the implementation should be specified before any further solutions can be found.

The trunk version introduced a new TOwnerDataListItems class, that can work only after a strict separation of non/OwnerData modes in all other methods. Currently exceptions are thrown in various places of the common code, when the control is switched into OwnerData mode. Again the handling of the list items and the widget interface has to be fixed, before this class can be made work.

How should the revised ListItems interface look alike, so that it can be made work in both non/OwnerData modes?

Or should we split the ListView into two dedicated controls, for normal and OwnerData lists and items, without allowing to switch OwnerData after creation of the control? Another variation could leave the widget in OwnerData mode - provided that this is supported by all widgetsets. Then the items must not be duplicated in normal mode, and nothing changes in OwnerData mode with user-supplied items.

DoDi

Hello,

First of all i appologize for being out-of-community and answering this laet. my primary job is keeping me busy.

But anyway. There's only one real solution - make a stand-alone control. And make it "non-owned" data (without any options), as it should be. Using System themes can make it look exactly like a system control, so no issues on "native look". Also, creating such control automatically gives the implementation for any possible widgetset.

As for ListView as it is now. I planned to add TOwnerDataListItems, when I implemented OwnerData for Win32 in the first place, just to prevent more bugs to be reported, as well as easier "index" handling. So, I don't think it should go away, the bugs that it is causing should be addressed.

thanks,
Dmitry

First of all i appologize for being out-of-community and answering this laet. my primary job is keeping me busy.

No problem ;-)

Sorry, I don't understand your idea. Do you vote for implementing another control for OwnerData, like TVirtualTreeView?

As for ListView as it is now. I planned to add TOwnerDataListItems, when I implemented OwnerData for Win32 in the first place, just to prevent more bugs to be reported, as well as easier "index" handling.

IMO there exists no need for "index" handling - as long as the widget can supply all requested information, in every widgetset. Keeping a local ItemList in sync with the widget currently is causing much trouble :-(

I'd favor a single GetItem method, that handles the possible states:
- OwnerData -> get item using OnData
- no widget created -> get item from ListItems
- widget exists -> read item from widget

An item cache is required *only* for OwnerData, to optimize the display of the items. Without such an cache, OnData has to be called for every single property, requested by the widget. In non-ownerdata mode no such requests occur, since the widget manages all item data itself.

So, I don't think it should go away, the bugs that it is causing should be addressed.

Most bugs result from wrong handling of OwnerData, in the common code. I'm willing to fix these bugs, but only after we agree about a valid model for handling both OwnerData states (T/F).

DoDi

Hans-Peter Diettrich wrote

Sorry, I don't understand your idea. Do you vote for implementing another control for OwnerData, like TVirtualTreeView?

Not only for OwnerData it would be useful, it could also be used for icon style in Carbon and could be used for Cocoa too.

I.e. the current implementation should be extended or replaced by a better implementation?

I'd second that idea, currently I'm working on such an improvement for tabbed controls. The TCustomNotebook suffers from the added pages, which should be added only in a derived control, as special kind of owner data.

DoDi

Hans-Peter Diettrich wrote

I.e. the current implementation should be extended or replaced by a better implementation?

it should make a check:

if treeview_style = vsIcon then UseCustomDrawnListView else UseNativeCarbonVersion

For newer widgetsets like LCL-Cocoa, lcl-fpgui, etc, I would simply just use the custom control version all the time to save time and headaches.

I'd second that idea, currently I'm working on such an improvement for tabbed controls. The TCustomNotebook suffers from the added pages, which should be added only in a derived control, as special kind of owner data.

Ummm, not really. Please note that TCustomNotebook is just a wrongly named component, it must have the tabs because huge amounts of code in the LCL depend on that. This component is scheduled for being renamed to TCustomPageControl.

It seams that you are mixing TCustomNotebook with TNotebook, but they are completely unrelated. TCustomNotebook is almost the same as TPageControl and it is a native control. TNotebook is a custom drawn control.

Is it something related in the process that prevents r31548 from compiling on Linux

gtk2wscomctrls.pp(162,43) Error: Identifier not found "TSortDirection" gtk2wscomctrls.pp(161,21) Error: There is no method in an ancestor class to be overridden: "class TGtk2WSCustomListView.SetSort(const TCustomListView,const TSortType,const LongInt,const <erroneous type>);"
gtk2wscomctrls.pp(239,1) Fatal: There were 2 errors compiling module, stopping Fatal: Compilation aborted

Works fine here. Maybe you changed something in your svn copy ? ComCtrls (there's TSortDirection declared) should be included in gtk2wscomctrls. Lazarus 0.9.31 r31551 FPC 2.4.3 i386-linux-gtk 2

zeljko
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus.freepascal.org/pipermail/lazarus/attachments/20110704/4f299a17/attachment.html>

Why ? because nobody tried to fix vsSmallIcon and vsIcon under carbon ?

zeljko

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus.freepascal.org/pipermail/lazarus/attachments/20110704/58323502/attachment-0001.html>

2011/7/4 zeljko :

Everything works, it was probably my fault, seems I updated and made in a subfolder

2011/7/4 zeljko :

Why ? because nobody tried to fix vsSmallIcon and vsIcon under carbon ?

Carbon has no API to implement vsIcon, the same for Cocoa.

Finder has something that works similar to vsIcon, although with some differences, but it is not implemented using publicly available Carbon nor Cocoa APIs.

I already searched the carbon-dev and cocoa-dev mailling lists and tons of people ask how to implement something like vsIcon, the simple answer every single one of them got was: there is no available API, you have to roll your own

But with qt under mac it works, so it's possible (only in case if qt doesn't also makes custom draw of vsIcon and vsSmallIcon).

zeljko
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus.freepascal.org/pipermail/lazarus/attachments/20110704/7751c3f7/attachment.html>

The widgetsets have to be updated accordingly, once we agree about implementing sorting in general. I'm only experienced with the Win32 widgets, and don't know how other widgetsets support ListViews and exist two general approaches WRT to the item lists:
1) either have everything stored and managed by the widget (Delphi),
2) or do everything in the LCL, leaving the widgets in OwnerData mode.

The current LCL implementation tries to do both at the same time, with immense trouble in keeping both lists (LCL and widget) in sync. Delphi manages this more clever, using [1] and a temporary list of items before or between eventually required RecreateWnd calls. The "virtual" approach [2] might be more appropriate with dumb widgetsets, which herefore only must allow for an virtual (OwnerData) mode. Then a true OwnerData control could be implemented as the LCL base class, with a derived class that also implements an item list. Then all additional features can be implemented in the LCL, the widgetsets must not be touched any more.

DoDi

Yes, I realized that misinterpretation already. If somebody is willing to update the widgetsets accordingly, I can do all the required LCL and IDE updates right now. (see below)

It seams that you are mixing TCustomNotebook with TNotebook, but they are completely unrelated.

No, I don't confuse these, but I admit that something should be renamed.

TCustomNotebook is almost the same as TPageControl and it is a native control.

That's not true, at least not for Win32. The native Win32 TabbedCtrl has no idea of pages, that's a Delphi/Lazarus addition that could have been implemented in a *derived* control.

Now for the good news:

I just finished the revised implementation of TCustomNotebook, so that it can work with and without pages. In that approach I didn't rename TCustomNotebook, because this is the type used in the widgetsets (see above). But I made some methods abstract, and derived two new (intermediate) classes:

- TPagedNotebook is the old implementation, for use in e.g. TPageControl. I've redirected all derived controls in the LCL to this new class. The IDE continues to work, no new bugs seem to be introduced.

- TCustomTabs/TTabs is the new tabbed control, without pages/tabsheets. This is a direct replacement for the T[Custom]TabControl, far more compatible with the Delphi implementation than the current implementation. Some properties still wait for their implementation in the various widgetsets.

I've left the old TTabControl in ComCtrls, for reference purposes during my development, and to allow for easier access to (currently) private elements in TCustomNotebook. The new TTabs can be renamed and moved there, after approval. The new TTabs does not require an additional page control, no separate page list, and no additional page controls, what reduces the resource usage (HWND...!) a lot.

I'm just thinking about makeing the Tabs.Objects[] optional pages, as outlined in the TabTest application. It doesn't make a difference to the tabbed control, whether it has associated (page/tabsheet) controls at all, and whether these controls are shown in its own client area or somewhere else. In fact TCustomPage only holds some per-page properties, in detail for communication with the widgetsets. TTabs uses a single dummy page internally, to keep some internal methods and the widgetsets happy.

Hans-Peter Diettrich wrote

Yes, I realized that misinterpretation already. If somebody is willing to update the widgetsets accordingly, I can do all the required LCL and IDE updates right now. (see below)

I already started that a long time ago, I already did the following steps:

1> Removed the old wrong TNotebook
2> Created a new correct TNotebook
3> Today I moved TCustomNotebook from ExtCtrls to ComCtrls

I don't understand what you mean here. TPageControl is not implemented like in Delphi, it does not inherit from the same control as TTabControl in the LCL.

My statement is correct: In the LCL TCustomNotebook is the same as TPageControl. Note that TPageControl is unrelated to TTabControl in the LCL

TCustomNotebook does not exist in Delphi.

See: http://docs.embarcadero.com/products/rad_studio/delphiAndcpp2009/HelpUpdate2/EN/html/delphivclwin32/ComCtrls_TPageControl.html

It seams that you are proposing to change that, I think the idea is interresting, fixing the object hierarchy to be the same as in Delphi. But be prepared for a lot of work =)

Have you already commited? It seams like this will conflict big time with my changes if you started before them =)

- TPagedNotebook is the old implementation, for use in e.g. TPageControl. I've redirected all derived controls in the LCL to this new class. The IDE continues to work, no new bugs seem to be introduced.

ummm .... some problems with that:

1> Please don't name anything related to tab controls as notebook

2> Are you sure that simply using the Delphi hierarchy here couldn't be a good choice?

3> Does it work in all widgetsets?

Okay so far, I just realized that Delphi also has the tabbed controls in ComCtrls.

Right, that's a big mistake in the LCL :-(

My statement is correct: In the LCL TCustomNotebook is the same as TPageControl. Note that TPageControl is unrelated to TTabControl in the LCL

The LCL TTabControl *erroneously* uses *another* TPageControl internally, only to show its tabs! Instead it should be based immediately on the widget that shows the tabs, e.g. the Windows TabCtrl.

TCustomNotebook does not exist in Delphi. See: http://docs.embarcadero.com/products/rad_studio/delphiAndcpp2009/HelpUpdate2/EN/html/delphivclwin32/ComCtrls_TPageControl.html

Right, Delphi addresses the TabCtrl widget directly, where the LCL uses a TWSCustomNotebook widget.

It seams that you are proposing to change that, I think the idea is interresting, fixing the object hierarchy to be the same as in Delphi. But be prepared for a lot of work =)

It's not much work, but the <grmbl> LCL implementation of the TWSCustomNotebookClass made it hard to find out a simple solution :-(

Unfortunately I cannot provide my solution right now, due to your move of TCustomNotebook etc. into ComCtrls. Before I do more superfluous work, can you please specify how I should proceed:

Should I retain T[Custom]TabControl as is, and add another flyweight TTabs control,
or should I replace the old implementation by the new one?

Then you'll find in my patch this class hierarchy:

TCustomNotebook //should be renamed into TCustomTabCtrl TCustomTabs //the refactored TCustomTabControl
TTabs //the refactored TTabControl
TPagedNotebook //corresponds to the current T[WS]CustomNotebook TPageControl //etc., rebased but otherwise unchanged

IMO it were the right time then, to refactor and rename TWSCustomNotebook as well, to reflect above splitted base classes. In the simplest solution a few alternative methods should be added, for all methods which currently use an TCustomPage parameter. Or this class could be splitted in a new TWSTabCtrl base class and a derived TWSPagedCtrl class.

DoDi

I've not supplied the patch yet, have to move everything into ComCtrls first :-(

The widgetsets refer to TCustomNotebook, that's why I cannot change that name :-(

2> Are you sure that simply using the Delphi hierarchy here couldn't be a good choice?

That's just what I implemented: the base class provides a tabbed control without pages, the derived classes add pages (tabsheets...) to it.

3> Does it work in all widgetsets?

A few modifications may be required, where the widgetset implementation assumes that a TCustomPage parameter is for use as a visible control. I've found two methods in the Win32 widgetset, which have to bypass some work, based on
if (ANotebook is TPagedControl) then {handle page visibility...} Otherwise the TCustomPage parameter only contains tab attributes, e.g. title = page.Caption.

I do not really understand the complicated handling of the page controls, in both LCL and widgetset code. As demonstrated in a simple test program for TTabControl, eventually related controls (tabsheets...) can become ordinary child controls of the basic tabbed control, with only the selected one being made visible. This can be done with other controls, too, which can have any suitable Parent (maybe Nil for detached floating forms). This approach will also allow to dock controls of any kind into a page control, as the EasyDockManager does internally.

DoDi

I think you should do it like this:

General concepts:
1> Avoid changing the current TPageControl as much as possible, because it is extensively used and changes here might cause a pletora of problems
2> I think it is no problem to introduce heavy changes to TTabControl because it is rarely used
3> Use exactly the same class hierarchy as Delphi:

TCustomNotebook (in Delphi called TCustomTabControl) --> TPageControl
--> TTabControl

However I would do one thing differently from your idea: Let TCustomTabControl still have a notion of pages so that it can fully implement TPageControl, but allow it to be able to display tabs without pages too. Hide page properties and let TPageControl increase their visibility.

Just because this hard distinction of a control with pages and one without makes sense in Windows it doesn't mean at all that it makes sense in all widgetsets. Plus, just 1 implementation is better then 2 spread across various classes for the widgetset interfaces.

And I would make smaller patches implementing the following actions:

1> Make sure TCustomNotebook can implement TTabControl, add an option that it can work without tabs and test in all widgetsets (maybe it can already do that, you would need to check this)
2> Change TTabControl to inherit from TCustomNotebook 3> Fix all problems found in the new TTabControl
4> Remove the current completely wrong class TCustomTabControl 5> Rename TCustomNotebok to TCustomTabControl, remove all implementations relative to it in widgetsets

Also give some time so that other developers can say if they have a different oppinion on that.

That also was my design :-)

TCustomTabControl inherits (abstract) Pages from TCustomNotebook.

Just because this hard distinction of a control with pages and one without makes sense in Windows it doesn't mean at all that it makes sense in all widgetsets. Plus, just 1 implementation is better then 2 spread across various classes for the widgetset interfaces.

ACK, but the use of TCustomPage for Pages sucks.

As already mentioned, the EasyDockManager allows to dock any controls into its notebook, and there it's overkill to wrap every docked control into another page control. Instead the unused Tabs.Objects[] can hold any kind of controls, which can be used like pages, on demand.

I only can test with the available widgetsets (Win32, eventually gtk2)

2> Change TTabControl to inherit from TCustomNotebook 3> Fix all problems found in the new TTabControl

I'd leave the following steps for later, and for people which can test their changes to the widgetsets.

DoDi

I've just uploaded TabCtrl.patch, see Mantis #19575

DoDi

Please let's continue the discussion here, not in Mantis.

Obviously we differ very much in our concepts, so that we should either come to a consent, or leave everything as bad as is.

This hierarchy would require to make the widgetsets independent from specialized Pages (TCustomPage descendants), and this is just what I have in mind.

Just because this hard distinction of a control with pages and one without makes sense in Windows it doesn't mean at all that it makes sense in all widgetsets. Plus, just 1 implementation is better then 2 spread across various classes for the widgetset interfaces.

Do you know of any widgetset, whose tabbed control has an idea of related pages?

Now at your notes in Mantis #19575:

>>>>>>>>>>>>>>>>>>>> IMO it's overkill to base the widget on a TCustomNotebook - the internal representation of the page data is of no interest to the widgetset. A

simpler

base control would serve the same purpose, and would allow to implement a Delphi compatible TTabControl.

overkill is not a problem. The LCL can have more functionality then Delphi, what is a problem is having a component based on a wrong class hierarchy or having less functionality then Delphi. <<<<<<<<<<<<<<<<<<<<

You address the wrong class hierarchy of TTabControl, I assume? I fully agree that TTabControl should inherit from LCL TCustomNotebook.

>>>>>>>>>>>>>>>>>>>>>>>>>>>>

Ummm, I'm not sure about this patch. Some issues:

1> TPagedNotebook is a bad choice for a name, TPagedTabControl is better. <<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Feel free to rename it as you like.

>>>>>>>>>>>>>>>>>>>>>>>

2> You started by modifying TCustomNotebook ... I would much prefer if you started by fixing TTabControl instead.
<<<<<<<<<<<<<<<<<<<<<<<

Since TTabControl has no Pages, some methods and properties have to become at least virtual in TCustomNotebook.

Unlike in Delphi, where Pages are introduced only in TPageControl, the LCL TCustomNotebook *and* the widgetsets currently do not work without pages. But as I proved they *can* work without an PageList :-)

>>>>>>>>>>>>>>>>>>>>>>>

3> Your entire description of the bug seams to be based on a wrong understanding of how TTabControl should work. Why are you putting a TPanel inside a TTabControl??? TTabControl can only contain ... tabs. It can never contain a Panel nor a Page, nor anything else. <<<<<<<<<<<<<<<<<<<<<<<<<

You misunderstand the demo application. In contrast, why does a TPageControl put TabSheets into the tabbed widget???

The panels in the demo program only demonstrate that a TTabControl *can* have associated controls, *not* bound to TCustomPage derivates, and *not* bound to the TTabControl as their parent.

>>>>>>>>>>>>>>>>>>>

4> Why do you expect Tabs.Tabs.AddObject to work at all? You should not use this AFAIK, you should use TTabControl.Tabs.Add()

http://docs.embarcadero.com/products/rad_studio/delphiAndcpp2009/HelpUpdate2/EN/html/delphivclwin32/ComCtrls_TTabControl_Tabs.html [^]
<<<<<<<<<<<<<<<<<<<<

Please look at the sample program in that webpage - it does exactly this: hold related components (here an dialog) in the Objects[] property of the TTabControl.Tabs. This does *not* work with the LCL TTabControl, and this was the reason for my bug report.

Delphi TPageControl *also* uses the Tabs.Objects[] to hold the visible pages, associated with the tabs.

>>>>>>>>>>>>>>>>>>

I took a deeper look at the problem now and I am even more sure that you are starting at the wrong side of it.

TCustomNotebook cannot handle a tab control without pages at the moment, the very first thing should be implementing that in all widgetsets. I see two solutions:

1> Modify TCustomNotebook in all widgetsets so that it can handle a tab control without pages, in this case we would have:

-> TCustomNotebook <-- This class has a WS implementation ---> TPageControl
---> TTabControl
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

This essentiall is what I already did. Not in all widgetsets, but it should be easy to fix according problems everywhere. When a TCustomPage parameter is used only for passing further parameters to the widgetset, e.g. Page.Caption for the tab title, a widgetset should never try to deal with such an invisible (dummy) page - no window placement...

I used "ANotebook is TPagedNotebook" in the determination, whether the TabCtrl has associated *visible* pages. You can suggest any other notebook or page property for that purpose :-)

>>>>>>>>>>>>>>>>>>>>>>>>>>>>

2> Change our hierarchy to this:

TCustomTabControl <-- Without a WS implementation
--> TCustomNotebook <-- This class has a WS implementation -----> TPageControl
--> TTabControl <-- This class will gain a WS implementation <<<<<<<<<<<<<<<<<<<<<<<<

Since both paged and unpaged controls are based on the same widget, and have much functionality in common, I'd suggest instead:

TCustomTabControl <-- With basic WS implementation --> TCustomNotebook <-- With extended WS implementation -----> TPageControl
--> TTabControl

Then the TWSNotebook class can inherit from TWSTabControl.

Or associate TWSNotebook directly with TPageControl, however you like.

>>>>>>>>>>>>>>>>>>>>>>>>>>

The Delphi design of having a base class with tabs and a derivated one with pages is stupid and very windows-centric ... we won't be able to fully replicate that.
<<<<<<<<<<<<<<<<<<<<<<<<<<<

Sorry, that' nonsense :-(

*Every* tabbed widget can live without associated pages, at least it can be shrunk to only show the tabs and nothing else. This is what the current TTabControl does: show only the tabs of the embedded TCustomNotebook. But why use an embedded control for that purpose, when the widget already can do all that itself?

>>>>>>>>>>>>>>>>>>>

The closest solution is option 2, with the difference that TCustomTabControl will not be usable alone, because it won't have a WS implementation.
<<<<<<<<<<<<<<<<<<<<

That's the crappy workaround, currently used in TTabControl :-(

>>>>>>>>>>>>>>>>>>>>>>>>>>>>

Option 1 would require making public a lot of tab stuff into TPageControl and a lot of page stuff into TTabControl <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

Please give at least one concrete example.

Didn't I prove that a separation of paged from unpaged tabbed controls/widgets is feasable with little efforts?

I have already moved the affected stuff from TCustomNotebook into TPagedNotebook, and you can move that stuff from TPagedNotebook into TPageControl, and remove TPagedNotebook entirely, if you like.

DoDi