propose: scintilla userlist lineheight

[George Hummet]

Hey Guys..

currently scintilla uses the default style font settings when drawing userlists but uses a fixed size (9) for the lineHeight is userlists.
propose: it might make sense to relate the lineHeight to the used FontSize during userlist invokation.
Heres a patch with a (maybe Naive) implemention, but maybe there is more to adjust.

If thi is preferable, ill write the docs.
enjoy, Thorsten

--- ScintillaBase.cxx    2017-12-04 16:29:50.238126900 +0100
+++ ScintillaBase_m.cxx    2017-12-04 16:29:54.238336900 +0100
@@ -254,8 +254,11 @@
             return;
         }
     }
+    
+    int szEntry= (vs.styles[STYLE_DEFAULT].size / SC_FONT_SIZE_MULTIPLIER) +6;
+    
     ac.Start(wMain, idAutoComplete, sel.MainCaret(), PointMainCaret(),
-                lenEntered, vs.lineHeight, IsUnicodeMode(), technology);
+                lenEntered, szEntry, IsUnicodeMode(), technology);
 
     PRectangle rcClient = GetClientRectangle();
     Point pt = LocationFromPosition(sel.MainCaret() - lenEntered);

[Neil Hodgson]

George Hummet:

> currently scintilla uses the default style font settings when drawing userlists but uses a fixed size (9) for the lineHeight is userlists.

lineHeight is not fixed: it is calculated from the maximum height measured for all active styles and which is determined in ViewStyle::Refresh. Using the same line height in the list as in the document helps make it appear an integrated element of the document.

An argument could be made to use the height of just the font that will be used in the list. This will only ever make the list tighter than it currently is.

> propose: it might make sense to relate the lineHeight to the used FontSize during userlist invokation.
> Heres a patch with a (maybe Naive) implemention, but maybe there is more to adjust.
>
> If thi is preferable, ill write the docs.
> enjoy, Thorsten
>
> --- ScintillaBase.cxx 2017-12-04 16:29:50.238126900 +0100
> +++ ScintillaBase_m.cxx 2017-12-04 16:29:54.238336900 +0100
> @@ -254,8 +254,11 @@
> return;
> }
> }
> +
> + int szEntry= (vs.styles[STYLE_DEFAULT].size / SC_FONT_SIZE_MULTIPLIER) +6;

+6?

ViewStyle::Refresh executes a bunch of code that tries to discover the real size of text and any replacement should use the results.

Neil

[George Hummet]

   lineHeight is not fixed: it is calculated from the maximum height measured for all active styles and which is determined in ViewStyle::Refresh. Using the same line height in the list as in the document helps make it appear an integrated element of the document.

 

Hey Neal - I can confirm that only for PlatGTK - PlatWin uses a fixed initial Value of (edit:10px) at Line2073:

public: // static lineHeight
    ListBoxX() : lineHeight(10), fontCopy(0), technology(0), lb(0), unicodeMode(false),
        desiredVisibleRows(9), maxItemCharacters(0), aveCharWidth(8),
        parent(NULL), ctrlID(0), doubleClickAction(NULL), doubleClickActionData(NULL),
        widestItem(NULL), maxCharWidth(1), resizeHit(0), wheelDelta(0) {
    }

later, the utility function only adds a padding to aboves inital.

int ListBoxX::ItemHeight() const {
    int itemHeight = lineHeight + (static_cast<int>(TextInset.y) * 2); // Adds Padding
    const int pixHeight = images.GetHeight() + (static_cast<int>(ImageInset.y) * 2);
    if (itemHeight < pixHeight) {
        itemHeight = pixHeight;
    }
    return itemHeight;
}

 +6?

   ViewStyle::Refresh executes a bunch of code that tries to discover the real size of text and any replacement should use the results.

   Neil

 ...seems pretty reasonable- this should be better deducted from ViewStyle.

void ScintillaBase::AutoCompleteStart(int lenEntered, const char *list) {
(...)
const Point TextInset(6, 6); // Padding
int szEntry= (vs.styles[STYLE_DEFAULT].size / SC_FONT_SIZE_MULTIPLIER +(int)TextInset.y);
ac.Start(wMain, idAutoComplete, sel.MainCaret(), PointMainCaret(),
           lenEntered, szEntry, IsUnicodeMode(), technology);
(...)

Note: maybe it would be more useful, if Userlists in general would have an own Style, so Users have the freedom to adapt their styles to their needs.
enjoy, Thorsten

[Neil Hodgson]

George Hummet:

> Hey Neal - I can confirm that only for PlatGTK - PlatWin uses a fixed initial Value of (edit:10px) at Line2073:
>
> public: // static lineHeight
> ListBoxX() : lineHeight(10), fontCopy(0), technology(0), lb(0), unicodeMode(false),
> desiredVisibleRows(9), maxItemCharacters(0), aveCharWidth(8),
> parent(NULL), ctrlID(0), doubleClickAction(NULL), doubleClickActionData(NULL),
> widestItem(NULL), maxCharWidth(1), resizeHit(0), wheelDelta(0) {
> }
>
> later, the utility function only adds a padding to aboves inital.
> int ListBoxX::ItemHeight() const {
> int itemHeight = lineHeight + (static_cast<int>(TextInset.y) * 2); // Adds Padding
> const int pixHeight = images.GetHeight() + (static_cast<int>(ImageInset.y) * 2);
> if (itemHeight < pixHeight) {
> itemHeight = pixHeight;
> }
> return itemHeight;
> }

The initial value should always be reset in the creation method before display. By zooming the window and showing a list, you can tell that the list responds to the zoom so the initial value is not important.

In PlatWin.cxx, ListBoxX::Create:
void ListBoxX::Create(Window &parent_, int ctrlID_, Point location_, int lineHeight_, bool unicodeMode_, int technology_) {
…
lineHeight = lineHeight_;

GTK+ has always ignored the passed line height and deferred to the list widget's default sizing.

> Note: maybe it would be more useful, if Userlists in general would have an own Style, so Users have the freedom to adapt their styles to their needs.

Its possible, but its no longer easy to add a predefined style as all style numbers are allocated. A different style setting mechanism would need to be implemented, possibly allocating an extra element when required as is done for annotations with SCI_ALLOCATEEXTENDEDSTYLES.

Neil

[George Hummet]

> Note: maybe it would be more useful, if Userlists in general would have an own Style, so Users have the freedom to adapt their styles to their needs.

   Its possible, but its no longer easy to add a predefined style as all style numbers are allocated. A different style setting mechanism would need to be implemented, possibly allocating an extra element when required as is done for annotations with SCI_ALLOCATEEXTENDEDSTYLES.

   Neil

Hm. than it might seem appropiately to use the calltip styles. Attached a first Idea of an implementation which works for PLAT_WIN on my test setup.
enjoy, Thorsten

[George Hummet]

[Neil Hodgson]

George Hummet:

> Hm. than it might seem appropiately to use the calltip styles. Attached a first Idea of an implementation which works for PLAT_WIN on my test setup.

The patch is still using an incorrect height calculation - it is not calling DeviceHeightFont nor measuring the true height but it is adding an arbitrary 6 pixels. This means it doesn’t take considerations like screen scaling into account.
http://www.scintilla.org/NoScale.png

Choosing the calltip font and colours will change behaviour for current applications in a way that they have not opted into. A better approach would be to add an API to choose a style.

It takes away the border between the list and the document making it more difficult to understand what is happening.

For selected items, simple lists on Windows use white text over the selection colour to ensure it is visible. The patch uses the same text colour for both selected and non-selected items.

Here is what it looks like with my current SciTE setup after applying the patch:
http://www.scintilla.org/UserList.png

Complex lists on Windows, like file lists in Explorer, use a light version of the selection colour for the background allowing use of the same text colour for both selected and non-selected items. Allowing choice of this may be OK, but forcing it is not.

The patch does not compile because there is no STYLE_CALLIP - its STYLE_CALLTIP.

There hasn’t been any motivation for these changes. What is the actual goal here?

Neil

[George Hummet]

  For selected items, simple lists on Windows use white text over the selection colour to ensure it is visible. The patch uses the same text colour for both selected and non-selected items.

   Here is what it looks like with my current SciTE setup after applying the patch:
http://www.scintilla.org/UserList.png

woh- its the very first Idea :) so most of the topics have been already fixed.
Do you know an easy way to style the color borders ? as it might be desired to have them there should be an option implemented.
this is how it currently looks: https://github.com/arjunae/myScite/blob/devel/SciTE_screenshots/Scintilla_userList.PNG

[George Hummet]

woh- its the very first Idea :) so most of the topics have been already fixed.
Do you know an easy way to style the color borders ? as it might be desired to have them there should be an option implemented.
this is how it currently looks: https://github.com/arjunae/myScite/blob/devel/SciTE_screenshots/Scintilla_userList.PNG

.... just written a more improved early Development Version..using thickFrame is now (as before) the default,

to test, set   

# calltips
    style.*.38=fore:#000000,back:#DFDFDF

and change ScintillaBase.cxx

 258   bool useThickFrame=true; // change here to test. todo: Make user settable

enjoy,
Thorsten

[George Hummet]

Current Version here: https://github.com/arjunae/myScite/tree/devel/contrib/SciTE_patches/scintilla

[Neil Hodgson]

George Hummet:

+ std::unique_ptr<Surface> surfaceMeasure(Surface::Allocate(technology));
+ surfaceMeasure->Init(wParent.GetID());
+ surfaceMeasure->SetUnicodeMode(unicodeMode);
+ XYPOSITION deviceHeight = static_cast<XYPOSITION>(surfaceMeasure->DeviceHeightFont(size));
+ FontParameters fp(faceName, deviceHeight / SC_FONT_SIZE_MULTIPLIER, SC_WEIGHT_NORMAL, false, 0, technology, characterSet);
+ font.Create(fp);
+ int lineHeight = RoundXYPosition(surfaceMeasure->Height(font));
+ lb->Create(parent, ctrlID, location, lineHeight, unicodeMode, technology, useThickFrame);

All this processing is already performed inside ViewStyle.cxx and this code should be replaced with retrieving the line height from ViewStyle. The patch is also discarding attributes like bold and italics.

+ // Decision -> Shouldnt we just allocate style.40 and bump up val STYLE_LASTPREDEFINED=40 ?
+ if (!useThickFrame) acStyle ( vs.styles[STYLE_CALLTIP].size > 0) ? STYLE_CALLTIP : STYLE_DEFAULT;

STYLE_LASTPREDEFINED can not be changed as that would be strongly incompatible with current applications. Any new styles should be allocated by the application calling SCI_ALLOCATEEXTENDEDSTYLES.

Neil

[George Hummet]

   All this processing is already performed inside ViewStyle.cxx and this code should be replaced with retrieving the line height from ViewStyle. The patch is also discarding attributes like bold and italics.

 

Calltips implement that this way too, but i see your reason in arguing against that. For the attributes - had no problems either with bold and italics on the test setup.
Anyway, Currently im working to rework calltips and userlists to deduct lineheight from viewstyle. Looks like:

for the style.*40 argument - might be reaonable to think about extending the limits with scintillas current major.

enjoy, Thorsten

[Neil Hodgson]

George Hummet:

> for the style.*40 argument - might be reaonable to think about extending the limits with scintillas current major.

Which then disrupts all the style constants after 40. Also impacts other style numbering issues like active/inactive for the C++ lexer and substyles. This would be a huge amount of work and disruption for core Scintilla, lexers, and client code.

Neil