These are chat archives for AvaloniaUI/Avalonia

21st
Nov 2017
GoldyD
@GoldyD
Nov 21 2017 13:59

Hi!

Class Avalonia.Controls.AppBuilderBase<TAppBuilder>

private void SetupAvaloniaModules()
{
var moduleInitializers = from assembly in AvaloniaLocator.Current.GetService<IRuntimePlatform>().GetLoadedAssemblies()
from attribute in assembly.GetCustomAttributes<ExportAvaloniaModuleAttribute>()
where attribute.ForWindowingSubsystem == ""
|| attribute.ForWindowingSubsystem == WindowingSubsystemName
where attribute.ForRenderingSubsystem == ""
|| attribute.ForRenderingSubsystem == RenderingSubsystemName
group attribute by attribute.Name into exports
select (from export in exports
orderby export.ForWindowingSubsystem.Length descending
orderby export.ForRenderingSubsystem.Length descending

Double orderby is wrong

Nikita Tsukanov
@kekekeks
Nov 21 2017 14:01
OrderBy preserves original order for elements with same value
So double orderby is equalent to orderby+thenby
GoldyD
@GoldyD
Nov 21 2017 14:11
Pvs studio
Nikita Tsukanov
@kekekeks
Nov 21 2017 14:11
Meh, they don't know C#
These PVS studio guys are all about marketing
BTW, we are NOT putting their "dear pvs studio blablabla" at the beginning of every file

https://msdn.microsoft.com/en-us/library/bb534966(v=vs.110).aspx

This method performs a stable sort; that is, if the keys of two elements are equal, the order of the elements is preserved. In contrast, an unstable sort does not preserve the order of elements that have the same key.

Nikita Tsukanov
@kekekeks
Nov 21 2017 14:19
Not sure if @jkoritzinsky has meant it to be this way, though
Since that's equalent to
orderby export.ForRenderingSubsystem.Length, export.ForWindowingSubsystem.Length  descending
Which would mean that rendering subsystem will have more priority
But we probably don't care
GoldyD
@GoldyD
Nov 21 2017 14:22

You are mistake

Example:
using System;
using System.Linq;

public class Program
{

private static Tuple<int, int>[] aaa = new Tuple<int, int>[3] { Tuple.Create(1,2), Tuple.Create(2,1), Tuple.Create(1,3) };

public static void Main()
{
    var s = aaa.OrderBy(a => a.Item1).OrderBy(b => b.Item2).ToArray();
    for (var i=0; i<3; i++)
    {            
       Console.WriteLine(s[i].Item1.ToString() + "" + s[i].Item2.ToString());
    }
}

}
Result:
21
12
13

When change
var s = aaa.OrderBy(a => a.Item1).ThenBy(b => b.Item2).ToArray();
Result:
12
13
21

Nikita Tsukanov
@kekekeks
Nov 21 2017 14:23

aaa.OrderBy(a => a.Item1).ThenBy(b => b.Item2)

that would be aaa.OrderBy(a => a.Item2).ThenBy(b => b.Item1)

Order is reversed, see?
GoldyD
@GoldyD
Nov 21 2017 14:28
For reverse is seems good
Nikita Tsukanov
@kekekeks
Nov 21 2017 14:28
that sorting seems to be meant for processing modules with windowing/rendering subsystem requirements first
I'm not sure if we care if windowing or rendering ones get processed first
GoldyD
@GoldyD
Nov 21 2017 14:35
Why not use you variant with
orderby export.ForRenderingSubsystem.Length, export.ForWindowingSubsystem.Length descending
Nikita Tsukanov
@kekekeks
Nov 21 2017 14:36
That would be probably better, but will qualify as "formatting" change, that doesn't bring any value
Let's wait for @jkoritzinsky and see what that code was meant to do
GoldyD
@GoldyD
Nov 21 2017 14:43

Ok

I also found issue on ProgressBar.UpdateIsIndeterminate method
Code:
private void UpdateIsIndeterminate(bool isIndeterminate)
{
if (isIndeterminate)
if (_indeterminateAnimation == null || _indeterminateAnimation.Disposed)
_indeterminateAnimation = IndeterminateAnimation.StartAnimation(this);
else
_indeterminateAnimation?.Dispose();
}

It seems as brackets is missing after first if clause.

private void UpdateIsIndeterminate(bool isIndeterminate)
{
if (isIndeterminate)
{
if (_indeterminateAnimation == null || _indeterminateAnimation.Disposed)
_indeterminateAnimation = IndeterminateAnimation.StartAnimation(this);
}
else
_indeterminateAnimation?.Dispose();
}

Nikita Tsukanov
@kekekeks
Nov 21 2017 14:52
Hm...
This one is a good catch
We should probably require brackets everywhere
Jeremy Koritzinsky
@jkoritzinsky
Nov 21 2017 14:54
That code is meant to make a module with a windowing or rendering system requirement come before one without a requirement. For example, a D2D specific module imementarion loads instead of the generic implementation when using D2D.
Nikita Tsukanov
@kekekeks
Nov 21 2017 14:55
So either ordering is fine and we don't need to change that
Jeremy Koritzinsky
@jkoritzinsky
Nov 21 2017 14:55
Yep
GoldyD
@GoldyD
Nov 21 2017 14:55
Ok
GoldyD
@GoldyD
Nov 21 2017 15:05

Trivial issue in class Avalonia.Designer.Comm
Method Start

_comm = new CommChannel(_proc.StandardOutput.BaseStream, _proc.StandardInput.BaseStream);
Reversed parameters for output and input streams

Nikita Tsukanov
@kekekeks
Nov 21 2017 15:07
Ehm, are you sure that you know how Process class works?
And stdin/stdout in general
GoldyD
@GoldyD
Nov 21 2017 15:08
I see signature of constructor
public CommChannel(Stream input, Stream output)
Nikita Tsukanov
@kekekeks
Nov 21 2017 15:08
Yep
StandardOutput is the stream you can read from
not write to
It's an output stream in terms of the lauched process
GoldyD
@GoldyD
Nov 21 2017 15:10
You're right
I saw only names
Code quality is very high
Nikita Tsukanov
@kekekeks
Nov 21 2017 15:13
Thanks, that's nice to know
danwalmsley
@danwalmsley
Nov 21 2017 15:16
@kekekeks im noticing with dispatcher timer
var timer = new DispatcherTimer { Interval = TimeSpan.FromMilliseconds(5000) };
            timer.Tick += (sender, e) =>
            {
                var packet = new IdpPacket(6, IdpFlags.None);
                packet.Write((UInt16)0xA000);
                packet.Write(22.0f);
                packet.Seal();

                incomingData.Write(packet.Data, 0, packet.Data.Length);
            };

            timer.Start();
the first time it first
is after a 5 second delay
then after that
its like every 1ms
this is on linux
Nikita Tsukanov
@kekekeks
Nov 21 2017 15:21
I might have misiterpreted glib docs
danwalmsley
@danwalmsley
Nov 21 2017 15:24
hmmm i tried a simple repro and its ok
not sure whats happened
Nikita Tsukanov
@kekekeks
Nov 21 2017 15:27
Seems to be a bug in GLIB, actually
        var tmr = new DispatcherTimer()
        {
            Interval = new TimeSpan(5000)
        };
        tmr.Tick += delegate { Console.WriteLine("Tick " + DateTime.UtcNow); };
        tmr.Start();
danwalmsley
@danwalmsley
Nov 21 2017 15:27
did you manage to repro it?
Nikita Tsukanov
@kekekeks
Nov 21 2017 15:30
Wait, I'm doing that wrong
danwalmsley
@danwalmsley
Nov 21 2017 15:30
i tried a repro like yours and it was ok
Nikita Tsukanov
@kekekeks
Nov 21 2017 15:31
new TimeSpan(5000) isn't the same as TimeSpan.FromMilliseconds(5000)
Nope, it's ticking as intended
danwalmsley
@danwalmsley
Nov 21 2017 15:33
ok false alarm then
must be something in my code
sorry
it was a silly mistake !
ksigne
@ksigne
Nov 21 2017 15:57
hi guys! finally found my way to dig into avalonia
i've found that border.borderthickness is Thickness in WPF and double in Avalonia. that limits some design approaches
fix is easy, but is it needed in repo?
danwalmsley
@danwalmsley
Nov 21 2017 16:04
@ksigne someone is currently working on this...
AvaloniaUI/Avalonia#1281
the difficult part is when you have different thicknesses for each side, combined with corner radius
ksigne
@ksigne
Nov 21 2017 16:04
yes, it is. but actually i've never seen a case when these two are combined
danwalmsley
@danwalmsley
Nov 21 2017 16:05
true, it is rarely combined
ksigne
@ksigne
Nov 21 2017 16:05
so on a quick fix i'd rather split it to separate case
if (cornerRadius > 0 && borderThickness.Left > 0)
                {
                    context.DrawRectangle(new Pen(borderBrush, borderThickness.Left), rect, cornerRadius);
                }
                else
                {
                    if (borderThickness.Left > 0)
                    {
                        context.DrawLine(new Pen(borderBrush, BorderThickness.Left), rect.TopLeft, rect.BottomLeft);
                    }

                    if (borderThickness.Top > 0)
                    {
                        context.DrawLine(new Pen(borderBrush, BorderThickness.Top), rect.TopLeft, rect.TopRight);
                    }

                    if (borderThickness.Right > 0)
                    {
                        context.DrawLine(new Pen(borderBrush, BorderThickness.Right), rect.TopRight, rect.BottomRight);
                    }

                    if (borderThickness.Bottom > 0)
                    {
                        context.DrawLine(new Pen(borderBrush, borderThickness.Bottom), rect.BottomLeft, rect.BottomRight);
                    }
                }
danwalmsley
@danwalmsley
Nov 21 2017 16:07
I think you should submit a PR for that, and we can get the other PR to just add the case when you have radius?
ksigne
@ksigne
Nov 21 2017 16:08
this PR will eventually touch ContentPresenter as well
'cause for some reason ContentPresenter have it's own border drawing instead of template composition
idk if that is a technical debt or meant to be so
Steven Kirk
@grokys
Nov 21 2017 21:58
@ksigne i did that to try to reduce the depth of the visual tree - UWP also does it this way for the same reason
it would be best if you could work with the @vertexpipeline on #1281 for this, otherwise we'll end up with 2 conflicting PRs
Steven Kirk
@grokys
Nov 21 2017 22:15
hmm looks like our CI failures are probably caused by dotnet/standard#567
basically, more breakage in .net - not sure what we can do about it for the moment, we don't have any binding redirects
Steven Kirk
@grokys
Nov 21 2017 22:25
going to try an old image
danwalmsley
@danwalmsley
Nov 21 2017 23:04
@grokys ill try out #1284 asap