Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MaxWidth игнорируется, если у родителя HorizontalAlignment=Stretch #45

Closed
Athari opened this issue Mar 5, 2018 · 21 comments

Comments

@Athari
Copy link

Athari commented Mar 5, 2018

Пытаюсь разобраться с багом, с которым столкнулся в CsConsoleFormat. Так как систему лейаута я копировал вашу — только выкинул ненужное из-за одноразовости вывода и навёл гламуру, — то попытался сравнить с вашей реализацией, авось в процессе наведения гламура что-то сломал. С удивлением обнаружил ту же самую проблему.

Собственно, если установить HorizontalAlignment=Stretch родителю, то свойства MaxWidth и MaxHeight, уменьшающие размер относительно возвращаемого из MeasureOverride желаемого размера, игнорируются. Попытался воспроизвести подобное в WPF, там ограничение на размер действует всегда, за границы велезти не получилось.

Так как вы в этих лейаутах разбираетесь на порядок лучше меня, то хотелось бы услышать ваше мнение по поводу этого. Это баг?

Воспроизведение:

using ConsoleFramework;
using ConsoleFramework.Controls;
using ConsoleFramework.Core;
using ConsoleFramework.Native;
using ConsoleFramework.Rendering;

namespace ConsoleFrameworkApp
{
    class Program
    {
        static void Main()
        {
            var window = new Panel {
                Width = 7,
                Height = 2,
                HorizontalAlignment = HorizontalAlignment.Left,
                XChildren = {
                    new Fill {
                        HorizontalAlignment = HorizontalAlignment.Stretch,
                        Char = '_',
                        Attr = Attr.FOREGROUND_BLUE | Attr.FOREGROUND_INTENSITY | Attr.BACKGROUND_BLUE,
                        XChildren = {
                            new FillAlphabet {
                                AlphaWidth = 3,
                                AlphaHeight = 2,
                                MaxWidth = 2,
                                HorizontalAlignment = HorizontalAlignment.Left,
                                Attr = Attr.FOREGROUND_RED | Attr.FOREGROUND_INTENSITY | Attr.BACKGROUND_RED,
                            }
                        }
                    }
                }
            };
            ConsoleApplication.Instance.Run(window);
        }
    }

    class Fill : Panel
    {
        public char Char { get; set; }
        public Attr Attr { get; set; }

        public override void Render(RenderingBuffer buffer)
        {
            buffer.FillRectangle(0, 0, ActualWidth, ActualHeight, Char, Attr);
        }
    }

    class FillAlphabet : Fill
    {
        public int AlphaWidth { get; set; }
        public int AlphaHeight { get; set; }

        public FillAlphabet()
        {
            Char = '-';
        }

        protected override Size MeasureOverride(Size availableSize) => new Size(AlphaWidth, AlphaHeight);

        public override void Render(RenderingBuffer buffer)
        {
            base.Render(buffer);
            char nextChar = 'a';
            for (int y = 0; y < AlphaHeight; y++)
                for (int x = 0; x < AlphaWidth; x++)
                    buffer.SetPixel(x, y, nextChar++, Attr);
        }
    }
}

Ожидаемое:

ab_____
de_____

Действительное:

abc____
def____

Аналогичный тест из CsConsoleFormat.

@elw00d
Copy link
Owner

elw00d commented Mar 8, 2018

Привет ! Постараюсь посмотреть на днях, надеюсь, удастся вспомнить, как это работает :)

@elw00d
Copy link
Owner

elw00d commented Mar 8, 2018

Кажется, что система размещения работает корректно. Возможно, есть некоторые отличия от WPF в том, как работает Panel (так как я писал все контролы и контейнеры с нуля). Попытаюсь расписать подробно, что здесь происходит.

Рассмотрим этапы размещения конечного контрола -- FillAlphabet. Ограничения Min/Max_Width/Height используются системой размещения на этапе Measure. Вот здесь:

MinMax mm = new MinMax(MinHeight, MaxHeight, MinWidth, MaxWidth, Width, Height);

Размер области, доступной контролу для размещения, вычисляется с учётом этих ограничений и передаётся на вход методу MeasureOverride(). И действительно, этот метод вызывается с аргументом (2, 2):

Но предложенная реализация MeasureOverride игнорирует переданные аргументы и возвращает в любом случае (3, 2). Соответственно, далее этот размер запоминается как минимально необходимый для отрисовки контрола (кстати, DesiredSize будет-таки обрезан до (2, 2), чтобы дать возможность родительским контролам разместить его с учётом его Min-Max ограничений). После этого управление будет передано контролу уровнем выше -- панели Fill. Что она увидит ? Что дочерний контрол имеет DesiredSize=(2, 2), но панели-родителю места по горизонтали доступно чуть больше - 7, и она размещает его (зовёт Arrange()) в слоте размером (7, 2) согласно логике размещения при Orientation.Vertical. При этом ArrangeOverride() у FillAlphabet зовётся с аргументом (3, 2) -- с тем размером, который контрол вернул из MeasureOverride. Этот же размер будет передан методу Render(). Render отрисует 3х2 в локальный буфер контрола. А родительская панель разместит его в слоте (7,2).

Что можно сделать с этим ? Есть два пути.

  1. Исправить логику FillAlphabet. Например, сделать реализации MeasureOverride чувствительной к availableSize, а Render -- чувствительной к this.ActualSize (см другие контролы).
  2. Попытаться изменить логику размещения панели -- можно переделать её реализации MeasureOverride/ArrangeOverride так, чтобы она не выдавала дочерним контролам слотов, превышающих их же DesiredSize. Но в этом случае размещение дочерних элементов может пострадать в других кейсах.

В общем, я рекомендую попробовать следовать по первому пути. Вот такой код FillAlphabet приведёт к ожидаемому результату:

class FillAlphabet : Fill
{
    public int AlphaWidth { get; set; }
    public int AlphaHeight { get; set; }

    public FillAlphabet()
    {
        Char = '-';
    }

    protected override Size MeasureOverride(Size availableSize) {
        // Учитываем availableSize, соблюдаем ограничения
        return new Size(Math.Min(availableSize.Width, AlphaWidth),
            Math.Min(availableSize.Height, AlphaHeight));
    }

    public override void Render(RenderingBuffer buffer) {
        base.Render(buffer);
        char nextChar = 'a';
        for (int y = 0; y < Math.Min(AlphaHeight, buffer.Height); y++)
        for (int x = 0; x < Math.Min(AlphaWidth, buffer.Width); x++)
            buffer.SetPixel(x, y, nextChar++, Attr);
    }
}

Хотя, может быть, второй путь тоже верный (надо попробовать).

Здесь есть немного текста про то, как работает система размещения

https://github.com/elw00d/consoleframework/blob/37fd4b1f2a2573342828f2424abf26ce1526ef34/docs/ru/LayoutSystem.txt

@Athari
Copy link
Author

Athari commented Mar 8, 2018

@elw00d Первый подход (исправить жадный контрол) я считаю костыльным, потому что по правилам WPF контрол может попросить размер больше, и это будет корректно обработано. Любое отличие от исходной логики размещения в WPF, которое не имет серьёзного обоснования, я склонен считать багом.

Кроме того, меня сильно смущает, что на учёт свойства MaxWidth потомка влияет выравнивание родителя (если у Fill установить HorizontalAlignment = HorizontalAlignment.Left, то FillAlphabet обрежется до (2; 2)). В этом мне видится что-то нелогичное.

Второй подход (изменить логику панели) тоже не без проблем. Логика размещения контролов в вашем Panel практически один-в-один повторяет логику в StackPanel из WPF. Если такая логика размещения работает в WPF, то, наверное, она должна работать и в консоли.

Положим, для моей библиотеки поддержка этого сценария не столь критична: обрезание отрендеренного текста — проблема сама по себе, потому что нет никакой возможности увидеть обрезанный кусок. Однако в случае вашей библитеки это имеет большее значение, потому что возвращение размера больше, чем дали — это основа работы скроллбаров в ScrollViewer, масштабирования в Viewbox и т.п. (Понимаю, что это больше теоретический вопрос, учитывая статус проекта, но всё же.)

Так как DesiredSize — единственное свойство, которое после всех вычислений сохраняет область, которая реально должна отобразиться, я пытался впихать его в вычисление отображаемой области контрола, но все мои попытки ломали рендеринг чуть менее, чем полностью. Так как я не очень представляю, как эта область должна вычисляться, то у меня сейчас вообще нагромождение пересечений, подобранное методом научного тыка:

Rect clip = new Rect(element.RenderSize)
    //.Intersect(new Rect(element.DesiredSize))
    .Intersect(element.LayoutClip)
    .Offset(offset)
    .Intersect(renderRect)
    .Intersect(parentRect);

Там что на самом деле должно быть-то? Х) (У меня только один буфер ConsoleBuffer, свойство Rect Clip меняется рекурсией рендерера.)

@elw00d
Copy link
Owner

elw00d commented Mar 8, 2018

Первый подход (исправить жадный контрол) я считаю костыльным,

Этот подход не костыльный, потому что в MeasureOverride контрол должен сообщать, сколько ему реально нужно места для отрисовки, с учётом переданного availableSize (а не игнорируя availableSize полностью). Кажется, я не предлагал использовать MaxWidth в этом месте (оно будет учтено автоматически при подготовке availableSize).

потому что по правилам WPF контрол может попросить размер больше, и это будет корректно обработано.

Здесь нет противоречия. Как я и описывал, случай, при котором контрол может запросить размер больше предлагаемого, существует, и он обрабатывается корректно. Собственно, изначальный пример и демонстрирует это (контрол не обрезается, а выводится полностью). Всё зависит от кода родительского контейнера (и его режима Stretch, как мы выяснили 😺)

Любое отличие от исходной логики размещения в WPF, которое не имет серьёзного обоснования, я склонен считать багом.

Да, возможно, вы правы и где-то есть расхождения в логике... Но это не отменяет рекомендаций 😺 На самом деле код основных функций Measure и Arrange взят из исходников WPF (с некоторыми упрощениями, если я правильно помню). Там даже комментарии остались оригинальные. Посему винить в чём-то именно этот кусок я бы не стал. Опять же, склонен предполагать, что если и есть различие в алгоритмах, то именно в логике панели.

Кроме того, меня сильно смущает, что на учёт свойства MaxWidth потомка влияет выравнивание родителя (если у Fill установить HorizontalAlignment = HorizontalAlignment.Left, то FillAlphabet обрежется до (2; 2)). В этом мне видится что-то нелогичное.

Это влияет не на учёт MaxWidth, это влияет на то, как панель ведёт себя с дочерними контролами в целом. При StretchAlignment в коде начинают отрабатывать ветки, которые не работают для других режимов:

// Alignment==Stretch --> arrange at the slot size minus margins
// Alignment!=Stretch --> arrange at the unclippedDesiredSize 
if (HorizontalAlignment != HorizontalAlignment.Stretch) {
    arrangeSize.Width = unclippedDesiredSize.Width;
}

if (VerticalAlignment != VerticalAlignment.Stretch) {
    arrangeSize.Height = unclippedDesiredSize.Height;
}

конкретно в вашем случае это приводит к тому, что дочерний контрол получает ровно (2,2) и ни дюймом больше.

И это опять же часть оригинального кода WPF. Поэтому сомневаться в ней я бы стал в последнюю очередь 😺

Однако в случае вашей библитеки это имеет большее значение, потому что возвращение размера больше, чем дали — это основа работы скроллбаров в ScrollViewer, масштабирования в Viewbox и т.п.

Со сложными контейнерами типа ScrollViewer, ComboBox, ListBox, GroupBox, Grid, TabControl было довольно много возни, но пока что при их создании я не обнаружил неразрешимых концептуальных проблем в системе размещения. Поэтому ещё раз повторюсь, я склонен больше доверять текущей системе размещения, а в случае проблем подозревать скорее код контейнеров (написанный с нуля и без оглядки на WPF).

Там что на самом деле должно быть-то?

Возможно, вам поможет описание инвариантов рендеринга в начале документа https://github.com/elw00d/consoleframework/blob/37fd4b1f2a2573342828f2424abf26ce1526ef34/docs/ru/LayoutSystem.txt

@elw00d
Copy link
Owner

elw00d commented Mar 9, 2018

Сегодня поковырялся ещё раз, попробовал в WPF, проанализировал код consoleframework, и пришёл к выводу, что скорее всего вы всё же правы. В оригинальном коде системы размещения WPF есть код, который учитывает это

//clippedInkSize differs from InkSize only what MaxWidth/Height explicitly clip the
//otherwise good arrangement. For ex, DS<clientSize but DS>MaxWidth - in this
//case we should initiate clip at MaxWidth and only show Top-Left portion
//of the element limited by Max properties. It is Top-left because in case when we
//are clipped by container we also degrade to Top-Left, so we are consistent.
Size clippedInkSize = new Size(Math.Min(innerInkSize.Width, mm.maxWidth),
    Math.Min(innerInkSize.Height, mm.maxHeight));

и далее

//remember we have to clip if clientSize limits the inkSize
NeedsClipBounds |=
    DoubleUtil.LessThan(clientSize.Width, clippedInkSize.Width)
        || DoubleUtil.LessThan(clientSize.Height, clippedInkSize.Height);
 
    Vector offset = ComputeAlignmentOffset(clientSize, clippedInkSize);

а в реализации consoleframework эта штука учитывается только для расчёта смещения относительно слота, а на расчёт LayoutClip фактически не влияет. Попробуйте теперь ?

@Athari
Copy link
Author

Athari commented Mar 9, 2018

О, всё заработало. Починил и у себя. Спасибо за помощь!

@elw00d
Copy link
Owner

elw00d commented Mar 9, 2018

Обратите внимание ещё на 1 коммит:

e8d8bd9

c15c25a

(я тут поигрался с кнопочками, MaxWidth и Strech и нашёл ещё 1 баг). Но теперь, кажется, всё в порядке.

@Athari
Copy link
Author

Athari commented Mar 10, 2018

@elw00d А тест есть? В каком случае баг возникает?

Некисло кода добавилось, однако. Прежний getClippedClientSize у меня уместился в Size.Min(RenderSlotRect.Size, new Size(MaxWidth, MaxHeight)) - Margin, а с этим кодом такой фокус не прокатит. 😆

Меня несколько смущает, что выравнивание теперь считается в двух местах: computeAlignmentOffsetCore и calculateLayoutClip. Их никак нельзя совместить?

@Athari
Copy link
Author

Athari commented Mar 10, 2018

Погонял разные версии на вот этой разметке:

var window = new Panel {
    Width = 9,
    Height = 2,
    HorizontalAlignment = HorizontalAlignment.Left,
    XChildren = {
        new Fill {
            HorizontalAlignment = HorizontalAlignment.Stretch,
            Char = '_',
            Attr = Attr.FOREGROUND_BLUE | Attr.FOREGROUND_INTENSITY | Attr.BACKGROUND_BLUE,
            XChildren = {
                new FillAlphabet {
                    AlphaWidth = 8,
                    AlphaHeight = 3,
                    MaxWidth = 3,
                    HorizontalAlignment = HorizontalAlignment.Center,
                    Attr = Attr.FOREGROUND_RED | Attr.FOREGROUND_INTENSITY | Attr.BACKGROUND_RED,
                }
            }
        }
    }
};
  1. Старая версия (bde72d5) проглатывает обрезанный элемент. Баг.
  2. Промежуточная версия (e8d8bd9) выравнивает обрезанный элемент по центру в соответствии с обрезанным размером. По-моему, это самая правильная версия.
  3. Текущая версия (c15c25a) выравнивает обрезанный элемент по центру в соответствии с запрошенным размером, в результате элемент оказывается смещён вправо.

Попробовал воспроизвести похожее в WPF — вроде, получается второй вариант.

<Border Background="Yellow" Width="800" Height="200">
    <Border Background="LightSkyBlue" MaxWidth="400" HorizontalAlignment="Center">
        <Border Background="Orange" Width="600"/>
    </Border>
</Border>

@elw00d
Copy link
Owner

elw00d commented Mar 10, 2018

Я исходил из следующих соображений:

  1. Нельзя менять размер, передаваемый в Arrange -- это нарушило бы протокол
  2. Нельзя напрямую изменять clientSize -- окно, через которое родительский элемент смотрит на дочерний (то, что я сделал сначала). Потому что после этого результаты вычислений layoutClip и применения марджинов портятся, и непонятно, как их можно исправить.
  3. Остаётся такой вариант: после определения размера layoutClip и видимой области в рамках него (что получается наложением layoutClip на (0, 0, renderSize.W, renderSize.H)) есть некоторая область visibleLayoutClip. Она подготовлена с учётом марджинов и остальных ограничений, и кажется наиболее логичным применить клиппинг по MaxWidth именно здесь. Причём можно применить его в зависимости от Alignment контрола так, что он будет занимать не более MaxWidth пикселей слева/справа/по центру. После экспериментов с кнопочками в разных режимах и анализа тестов я решил, что это именно то, что подойдёт в этой ситуации. Во всяком случае, это поведение больше всего похоже на ожидаемое, и вполне объяснимо. Ну если выясню, что WPF работает иначе, то подправлю. Пока что не смог заставить исходный код WPF работать в дебагере (хотя раньше это работало), и к сожалению это сильно мешает полноценному исследованию.

@elw00d
Copy link
Owner

elw00d commented Mar 10, 2018

Текущая версия (c15c25a) выравнивает обрезанный элемент по центру в соответствии с запрошенным размером, в результате элемент оказывается смещён вправо.

Кажется, это так только если HorizontalAlignment=Stretch в дочернем контроле. Если поставить Center, то будет по центру. Попробуйте ?

@Athari
Copy link
Author

Athari commented Mar 10, 2018

Кажется, это так только если HorizontalAlignment=Stretch в дочернем контроле. Если поставить Center, то будет по центру. Попробуйте ?

Но это же нелогично. Я хочу выровнять по центру ребёнка, а не родителя. Родитель у меня для фона.

@elw00d
Copy link
Owner

elw00d commented Mar 10, 2018

Недопонял, поясните пожалуйста подробнее

@Athari
Copy link
Author

Athari commented Mar 10, 2018

Почему для того, чтобы ребёнок корректно выровнялся по центру, он обязан быть обёрнут в родителя, выровненного по центру? Получается, выравнивание ребёнка по центру само по себе (без оборачивающего родителя) не работает.

Играюсь с маргинами. Да, у промежуточного варианта тоже не очень хорошо с выравниванием: симметричные маргины портят центрирование.

@elw00d
Copy link
Owner

elw00d commented Mar 10, 2018

Нет, я не то имел в виду, родитель не обязан быть выровнен по центру, это в дочернем контроле нужно указать Center.

@elw00d
Copy link
Owner

elw00d commented Mar 10, 2018

Играюсь с маргинами. Да, у промежуточного варианта тоже не очень хорошо с выравниванием: симметричные маргины портят центрирование.

Да, а с отрицательными марджинами вообще труба

@Athari
Copy link
Author

Athari commented Mar 10, 2018

Нет, я не то имел в виду, родитель не обязан быть выровнен по центру, это в дочернем контроле нужно указать Center.

В моём примере выше у ребёнка уже выравнивание по центру.

В общем, вот:

var window = new Panel {
    Width = 15,
    Height = 5,
    HorizontalAlignment = HorizontalAlignment.Left,
    XChildren = {
        new Fill {
            Margin = new Thickness(2, 1, 2, 1),
            HorizontalAlignment = HorizontalAlignment.Stretch,
            Char = '▪',
            Attr = Attr.FOREGROUND_BLUE | Attr.FOREGROUND_INTENSITY | Attr.BACKGROUND_BLUE,
            XChildren = {
                new FillAlphabet {
                    AlphaWidth = 8,
                    AlphaHeight = 3,
                    MaxWidth = 5,
                    Margin = new Thickness(1, 0, 1, 0),

                    // Не работает в исходной версии: слишком большая ширина
                    HorizontalAlignment = HorizontalAlignment.Left,
                    // Не работает в промежуточной версии: съезжает влево относительного правого края
                    HorizontalAlignment = HorizontalAlignment.Right,
                    // Не работает в последней версии: съезжает влево относительно центра
                    HorizontalAlignment = HorizontalAlignment.Center,

                    Attr = Attr.FOREGROUND_RED | Attr.FOREGROUND_INTENSITY | Attr.BACKGROUND_RED,
                }
            }
        }
    }
};

С отрицательными маргинами ещё не тестировал.

@Athari
Copy link
Author

Athari commented Mar 11, 2018

Нашёл недостающий кусок кода. Он находится в System.Windows.FrameworkElement.GetLayoutClip. В исходном варианте (все версии, кроме последней) ConsoleFramework.Controls.Control.calculateLayoutClip рассчитывает только аналог slotRect, а localRect остаётся без внимания. Плюс к этому размер у slotRect в оригинале считается по-другому, если я чего-то не пропустил.

В общем, я запилил Athari/CsConsoleFormat@e2a089f, там кроме исправления этой ситуации я ещё подчистил код, выкинул повторные расчёты одного и того же, выкинул комментарии, которые мне один чёрт никак не помогают, ну и ещё по мелочи. Я заменил расчёт LayoutClip на вот такую загогулину, и, вроде, всё заработало. Ну, по крайней мере два теста починилось, которые покрывают аналогичные недавно обсуждаемым конфигурации.

Короче, я ничего не гарантирую, потому что все эти вычисления остаются для меня неведомой мумбой-юмбой, но у меня работает. 😆

Не знаю, стоит ли тестировать отрицательные маргины, потому что, если они не работают, снова придётся в дебри кода WPF влезать... Проще объявить, что маргины бывают только положительные. 😁

@elw00d
Copy link
Owner

elw00d commented Mar 11, 2018

Рад, что у вас полечилось ) я поковыряюсь ещё на днях, но кажется, без отладки WPF это ни к чему не приведёт, а дебагер в VS 2017 сломан

завёл баг им, посмотрим, что ответят https://developercommunity.visualstudio.com/content/problem/212357/net-source-code-debugging-is-broken-in-vs-2017.html

@Athari
Copy link
Author

Athari commented Mar 11, 2018

Эта отладка сорцов хоть когда-нибудь работала вообще? По-моему, она работала от силы месяц после реализации, затем сломалась с первым обновлением фреймворка. Потом, спустя много времени, мелкомягкие что-то там наобещали про регулярное обновление сорцов даже с минорными релизами, оно снова где-то месяц проработало, затем снова сдохло. Может, оно и чаще работало, чем два раза за всё время существования, но я как-то не застал эти светлые моменты. 😆

Протестировал отрицательные маргины. Расчёт работает, вроде. Рендеринг пришлось слегка починить (Athari/CsConsoleFormat@198102e), но в ConsoleFramework такой проблемы не должно быть изначально.

@elw00d
Copy link
Owner

elw00d commented Mar 13, 2018

Да, я зря делал логику выбора позиции в зависимости от Alignment'а, т.к. уже в функции computeAlignmentOffset было ясно написано, что MaxWidth/Height будет отрезАть всегда слева и сверху:

//clippedInkSize differs from InkSize only what MaxWidth/Height explicitly clip the
//otherwise good arrangement. For ex, DS<clientSize but DS>MaxWidth - in this
//case we should initiate clip at MaxWidth and only show Top-Left portion 
//of the element limited by Max properties. It is Top-left because in case when we
//are clipped by container we also degrade to Top-Left, so we are consistent. 

убрал весь лишний код, отрезаю на Top-Left -- поведение стало согласованным с offset. Контрол начинает рисоваться посередине и обрезается слева. И тесты не сломались (хотя в одном месте пришлось исправить, т.к. после введения новых пересечений в layoutClip не может быть отрицательных чисел, то есть layoutClip стал по сути visibleLayoutClip, но на суть это не влияет).

Коммит d079561

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants