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.

Sign up for GitHub

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

Jump to bottom

Maui.Core Gtk enhancements #5

Merged
merged 13 commits into from
May 2, 2021
Merged

Maui.Core Gtk enhancements #5

merged 13 commits into from
May 2, 2021

Conversation

lytico
Copy link
Collaborator

@lytico lytico commented Apr 28, 2021

Maui.Core Gtk enhancements

Additions made

  • Platform
  • ILinuxLifecycle
  • MauiContext
  • MauiGtkApplication
  • MauiGtkMainWindow
  • FontManager

Handler-Enhancements

  • ActivityIndicatorHandler
  • ButtonHandler
  • CheckBoxHandler
  • EditorHandler
  • EntryHandler
  • LabelHandler

Prototype

  • LayoutHandler

@lytico lytico changed the base branch from main to main-linux April 29, 2021 11:13
@lytico lytico changed the base branch from main-linux to init-linux April 29, 2021 11:13
@lytico lytico mentioned this pull request Apr 29, 2021
Closed
jsuarezruiz
jsuarezruiz reviewed Apr 29, 2021
View reviewed changes
src/Core/src/Platform/Linux/LayoutView.cs
}

// this is maybe not needed:
void UpdateFocusChain()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will need it but probably at ViewHandler level.

jsuarezruiz
jsuarezruiz reviewed Apr 29, 2021
View reviewed changes
src/Core/src/Platform/Linux/MauiGtkApplication.cs Outdated

protected void OnWindowAdded(object o, WindowAddedArgs args)
{
;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will need this events to have notifications at cross platform Window level, but for now, we can remove it or just include a comment.

jsuarezruiz
jsuarezruiz reviewed Apr 29, 2021
View reviewed changes
src/Core/src/Platform/Linux/WidgetColorExtensions.cs
public static class WidgetColorExtensions
{

public static void SetBackgroundColor(this Gtk.Widget widget, Graphics.Color color)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.NET MAUI will have another object called Paint, that will give information about colors, gradients, etc. This extensions methods will be something like SetBackground(this Gtk.Widget widget, Graphics.Paint paint), but for now is fine.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this is related to have a Gtk specific Microsoft.Maui.Graphics implementation?
is it https://github.com/dotnet/Microsoft.Maui.Graphics/blob/02fe4e30a2ca482351e21fbb74cf44235a89c9e6/src/Microsoft.Maui.Graphics/Paint.cs?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly

jsuarezruiz
jsuarezruiz reviewed Apr 29, 2021
View reviewed changes
src/Core/src/Platform/Linux/WidgetColorExtensions.cs Outdated
public static Gdk.Color Dark(this Gtk.Style it, Gtk.StateType state)
{
// TODO
return it.Foreground(state);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would it be used?

Copy link
Collaborator Author

@lytico lytico Apr 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed, but i think i'll need it (or something similar) again to resolve OsAppTheme.Dark (https://github.com/lytico/maui/blob/622316ed64d88dca0da99d984abd155e7381f2af/src/Controls/src/Core/OSAppTheme.cs)

remark

some of WidgetColorExtensions are not used at this state of implementation
but i guess i'll need it later on

jsuarezruiz
jsuarezruiz reviewed Apr 29, 2021
View reviewed changes
src/Core/src/Platform/Linux/WidgetColorExtensions.cs Outdated
return it.Context.ColorFor("", state);
}

public static void ModifyBase(this Gtk.Widget it, Gtk.StateType state, Gdk.Color color)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would it be used?

jsuarezruiz
jsuarezruiz reviewed Apr 29, 2021
View reviewed changes
src/Core/src/[Microsoft.Maui.Graphics]/Linux/ColorExtensions.cs
@@ -0,0 +1,42 @@
namespace Microsoft.Maui.Graphics.Native.Gtk
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will ask about this. We have different options:

  • Create a Linux specific Microsoft.Maui.Graphics library here
  • Push the Linux changes directly to the Microsoft.Maui.Graphics repo.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please, your opinion to "Linux" versus "Gtk"; see: #6

second options would be better.

i'm planing to implement a Microsoft.Maui.Graphics Gtk NativeCanvas (if no others want to do it)

Copy link
Owner

@jsuarezruiz jsuarezruiz Apr 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mnn, right, Gtk also can run on macOS and Windows so, must be Gtk instead of Linux.

Copy link

@knocte knocte May 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there any point in running Gtk on Windows or macOS, though, especially if we're talking within the context of Maui? Because Maui's Windows and macOS support is a first-class citizen, while Gtk's first-class citizen is Linux (I myself found many Gtk bugs in Windows and macOS; so many, that we should discourage its use).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW guys you might want to get some inspiration from here (xamarin/Xamarin.Forms#14235) to implement Shapes API, a PR for XF that I just opened.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@knocte thanks for the hint!

to get some inspiration from here (xamarin/Xamarin.Forms#14235)

but for implementing maui - shapes, i would propose to have pure maui-renderers using https://github.com/dotnet/Microsoft.Maui.Graphics/blob/main/src/Microsoft.Maui.Graphics/ICanvas.cs for all platforms

is there any point in running Gtk on Windows or macOS

it could be a point having a Maui-platform KDE / Qt on linux, so changing "linux" -> "Gtk" makes sense anyway. None of the code is linux-specific by now, but Gtk-specific

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it could be a point having a Maui-platform KDE / Qt on linux,

It could, but QtSharp has been in alpha development for years (if not more than a decade) and no project uses it. It must be real hard to bind C++ from C#... So I would only rename 'linux' to 'gtk' when QtSharp+Maui really happens for real (which may be never).

jsuarezruiz
jsuarezruiz requested changes Apr 29, 2021
View reviewed changes
Copy link
Owner

@jsuarezruiz jsuarezruiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you rebase ini-linux to fix the conflicts?. Thanks!

@jsuarezruiz
Copy link
Owner

@lytico Nice progress, thanks!

@lytico
Copy link
Collaborator Author

lytico commented Apr 29, 2021

Could you rebase ini-linux to fix the conflicts?. Thanks!

yes, sorry, i've started before your commits.

but what MauiWindow is for?
https://github.com/jsuarezruiz/maui-linux/blob/5ce87eb9040e7bd513a152ca1c0b8a9d5f7722c3/src/Core/src/Platform/Linux/MauiWindow.cs

startup is done in
https://github.com/jsuarezruiz/maui-linux/blob/3cdb67f71c73d3b758516f5cde3a4819783d1f94/src/Core/src/Platform/Linux/MauiGtkApplication.cs

anyway, the whole maui startup pattern seems unclear at the moment.

should't it be something like

static void Main(args) {
...
     builder
            .UseMauiGtk()
...

}

@jsuarezruiz
Copy link
Owner

@lytico I was initialising the HostBuilder etc in the main Window. Can remove it for now and continue with the implementation at Application level.

@lytico lytico mentioned this pull request Apr 30, 2021
Closed
@lytico
Copy link
Collaborator Author

lytico commented Apr 30, 2021

sorry, can't rebase (don't understand why). git refuses to pull the rebased branch

@jsuarezruiz jsuarezruiz merged commit b045453 into jsuarezruiz:init-linux May 2, 2021
@lytico lytico mentioned this pull request May 10, 2021
Open
@github-actions github-actions bot locked and limited conversation to collaborators Apr 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@knocte knocte knocte left review comments

@jsuarezruiz jsuarezruiz jsuarezruiz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lytico @jsuarezruiz @knocte