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

[W10] CoreApplication project template (VS2015) & fixes #5615

Merged
merged 2 commits into from
Nov 2, 2017

Conversation

nkast
Copy link
Contributor

@nkast nkast commented Mar 23, 2017

(requires fix #5613)

Related to #5915

@SimonDarksideJ
Copy link
Contributor

Another template in the VS2015 folder @nkast ?
Also, must make sure this is also tested on a clean VS2017 install (don't want a repeat of the template problems in 3.6 base)

@nkast
Copy link
Contributor Author

nkast commented Mar 24, 2017

The template is similar to the Windows 8.1 template for CoreApplication in VS2013 but for W10 which was missing.
I don't have VS2017.

@SimonDarksideJ
Copy link
Contributor

So how is it different to the existing Windows 10 UWP template in the VS2015 folder? In other words, why new instead of update?

@nkast
Copy link
Contributor Author

nkast commented Mar 24, 2017

The existing template uses the XAML framework and the new template runs on a much slimmer CoreApplication framework. Both types of apps have their pros and cons.

XAML for example let's you mix XAML control on top of the backbuffer, ADvertising libraries for example.
The non-xaml/ CoreApplication app has better performance, shorter startup time and uses less memory.

@SimonDarksideJ
Copy link
Contributor

I agree, we should have both XAML and D3D templates for Win10, so I would welcome this addition.
Even better if we can do away with the redundant Win8.1/Phone8.1 templates to clean it up

@nkast
Copy link
Contributor Author

nkast commented Sep 1, 2017

I don't see how it's related to this PR but since you mention it, I welcome the decision to keep W8.1 for at least one more release. MG needs to be able to claim that it had a working W8.1 version, the platform was more or less broken on the previous releases.

@SimonDarksideJ
Copy link
Contributor

Except @nkast that Win8.1 has many outstanding issues, all of which have not been resolved and almost nothing has been done for the platform since 3.6.
So removing it from 3.7 and leaving 3.6 in its current state has no difference afaik.
Plus there are no plans to work on or fix Win8.1 post 3.7, which if it's dropped in 3.8 would mean the same thing. Nothing new since 3.6 for the platform.

The fact even MS has dropped support should merit dropping it from 3.7 in favour of a newer version of SharpDX

@SimonDarksideJ
Copy link
Contributor

Yo @nkast
Is this PR valid for UWP ? Seems we are entering an age where this would be extremely useful, like for VR and other "true" 3D titles.

@nkast
Copy link
Contributor Author

nkast commented Oct 30, 2017

I fixed the conflict. This should work now.

@SimonDarksideJ
Copy link
Contributor

Thanks @nkast for the fast response.

@nkast
Copy link
Contributor Author

nkast commented Oct 30, 2017

Remember that you need to combine it with #5613 for a runtime test to work.

@nkast nkast changed the title [W10] CoreApplication project template & fixes [W10] CoreApplication VS2015 project template & fixes Oct 30, 2017
@nkast nkast changed the title [W10] CoreApplication VS2015 project template & fixes [W10] CoreApplication project template (VS2015) & fixes Oct 30, 2017
@SimonDarksideJ
Copy link
Contributor

SimonDarksideJ commented Oct 30, 2017

When tested with Win10 / VS2107 and SDK 16299, it crashes on this line 213 of InputEvents.cs
As Window.Current is null on startup, the same code is called from 3 different events. SizeChanged, VisibilityChanged and Activated.

Suggest adding a "Window.Current != null" check around this code. (preferably refactored in a new function as it's called from 3 functions)

Otherwise, this code runs perfectly. Fantastic work @nkast

@tomspilman
Copy link
Member

@nkast @DDReaper - #5613 is merged now.

@SimonDarksideJ
Copy link
Contributor

Woot, then imho @tomspilman this is ready to merge now. Tested both D3D and XAML projects using the updates and it's all good.
Will follow up with a 2017 template once this is merged.

@nkast
Copy link
Contributor Author

nkast commented Oct 31, 2017

I rebased the PR to fix the conflict and include #5613.

As for the Window.Current.CoreWindow.IsInputEnabled = true; it was added in #5743, something about an odd bug in UWP. I think it's safe to skip it when Window.Current is null during startup.
FYI @roponator.

DDReaper Should I push the change in this PR?

@SimonDarksideJ
Copy link
Contributor

SimonDarksideJ commented Oct 31, 2017

I would recommend to add that null check, as in testing with 16299, it caused the game to halt
I would also recommend renaming the template to "D3D" instead of Core, simply as "core" doesn't seem to relate to anything, whereas you are providing direct access instead of XAML. In the C++ land, this is the D3D template.

@nkast
Copy link
Contributor Author

nkast commented Oct 31, 2017

It relates to CoreApplication.

@nkast
Copy link
Contributor Author

nkast commented Oct 31, 2017

Both projects use directx, this would be confusing. It sould be called Core or Lightweight or something like that and the other project XAML.

@nkast
Copy link
Contributor Author

nkast commented Oct 31, 2017

I named it Core to distinguish it from the "Windows Universal" (XAML) project.
In W8 the two projects were named "Windows Store Project" (CoreApplication) and
"Windows Store (XAML) Project" (XAML).

@nkast
Copy link
Contributor Author

nkast commented Oct 31, 2017

I made the change to InputEvents.cs and merged it into #23f0b22

Copy link
Contributor

@SimonDarksideJ SimonDarksideJ left a comment

Choose a reason for hiding this comment

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

Code changes now all work in release & debug modes for all UWP targets

@SimonDarksideJ
Copy link
Contributor

Over to you @tomspilman , this is ready to go. Will let you make the call on naming.
Code wise, this all works great.

@@ -1,8 +1,8 @@
<?xml version="1.0" encoding="utf-8"?>
<VSTemplate Version="3.0.0" xmlns="http://schemas.microsoft.com/developer/vstemplate/2005" Type="Project">
<TemplateData>
<Name>MonoGame Windows 10 Universal Project</Name>
<Description>A MonoGame game project for Windows 10 UWP.</Description>
<Name>MonoGame Windows 10 Universal (XAML) Project</Name>
Copy link
Member

Choose a reason for hiding this comment

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

I would say change this to:

MonoGame Windows 10 Universal XAML Project

@KonajuGames @dellis1972 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

So as to distinguish between the two templates, one being XAML and the other being a core / D3D application.
Just saying Win 10 Universal alone isn't enough to describe what it is

@tomspilman
Copy link
Member

@KonajuGames @dellis1972

To be clear this PR just adds support for running Windows 10 UWP apps that don't support XAML which are a little faster and use a little less memory.

Early on we decided we didn't need to support this... but i'm good with adding it now.

@KonajuGames
Copy link
Contributor

Merging. Thanks @nkast

@KonajuGames KonajuGames merged commit a02b4f8 into MonoGame:develop Nov 2, 2017
@SimonDarksideJ
Copy link
Contributor

Will get on the 2017 PR asap, hopefully before the weekend.

@nkast
Copy link
Contributor Author

nkast commented Nov 2, 2017

Thanks @KonajuGames !

@nkast nkast deleted the tnc_W10CoreApplication branch November 2, 2017 16:09
@Jure-BB
Copy link
Contributor

Jure-BB commented Jan 2, 2018

I get wrong backbuffer size on startup after merging this PR. (More info in #6129)

@nkast
Copy link
Contributor Author

nkast commented Jan 2, 2018

@Jure-BB

I get wrong backbuffer size on startup after merging this PR. (More info in #6129)

I don't see how. This PR is not related to backbuffer.

@Jure-BB
Copy link
Contributor

Jure-BB commented Jan 2, 2018

I think size changed event doesn't get fired at startup like it did before.

https://github.com/MonoGame/MonoGame/pull/5615/files#diff-4cee1f2a28f7ffd8b6168e8e93e9a840L129

@nkast
Copy link
Contributor Author

nkast commented Jan 2, 2018

The initialization code should create the backbuffer in the right size even if nothing get's resized.
Can you describe the issue in some more details? What dimensions are you getting for Window & backbuffer on startup? Is it in window mode? Are you setting the backbuffer via code on startup? (MG don't resize the window in UWP)

@nkast
Copy link
Contributor Author

nkast commented Jan 2, 2018

@Jure-BB
Maybe what you are seeing is similar to #5555?

Can you remove the following lines and see what happens?
https://github.com/MonoGame/MonoGame/blob/develop/MonoGame.Framework/GraphicsDeviceManager.cs#L69-L78

@Jure-BB
Copy link
Contributor

Jure-BB commented Jan 2, 2018

I did a mini test. Set breakpoint inside UAPGameWindow.cs SwapChain_SizeChanged & UpdateSize methods. In my project on older build both methods get called on startup but none in the latest build. I'll have to set up a clean project to so my code is not interfering with the test.

In windowed mode I get white area inside game windows as backbuffer is not the same size as viewbounds of the window.

@Jure-BB
Copy link
Contributor

Jure-BB commented Jan 2, 2018

Haven't dug deep into #5555, but this isn't affected by width/height ratio, if that's what you're asking. Width is greater than height in my tests. I'll try to create a sample project.

@nkast
Copy link
Contributor Author

nkast commented Jan 2, 2018

In windowed mode I get white area inside game windows as backbuffer is not the same size as viewbounds of the window.

That's another issue, #5914. Although the actual fix would be to resize the window as we do on other platforms.

@Jure-BB
Copy link
Contributor

Jure-BB commented Jan 2, 2018

Can I match backbuffer size to be the same as window viewbounds on startup without modifying MG?

@nkast
Copy link
Contributor Author

nkast commented Jan 2, 2018

I suspect that MG starts with 800x480 but the window stays the same, right?
I suppose you can read Game.Window.Bounds to set the initial graphics.PrefferedBackbufferWidth/Height.

@Jure-BB
Copy link
Contributor

Jure-BB commented Jan 2, 2018

Similar. I try to restore last used size, but the window stays the same.
Thanks, I try to read Game.Window.Bounds.

@Jure-BB
Copy link
Contributor

Jure-BB commented Jan 2, 2018

This was actually messing my code in a lot of places. So I disabled any resolution changes, by always overriding resolution to Game.Window.Bounds for UWP platform. Thanks @nkast for clarification!

@nkast
Copy link
Contributor Author

nkast commented Jan 2, 2018

Another idea,
You can try to set the window size yourself with something like this ApplicationView.GetForCurrentView().TryResizeView(...)
or have your app start at the default 800x480 with
ApplicationView.PreferredLaunchViewSize = new Size(800, 480);
ApplicationView.PreferredLaunchWindowingMode = ApplicationViewWindowingMode.PreferredLaunchViewSize;

@Jure-BB
Copy link
Contributor

Jure-BB commented Jan 2, 2018

That sounds like a good idea for PR. It would be nice, if UWP version of MG behaved similar to other platforms. I'll probably have a look at it when I'm closer to release.

nkast added a commit to nkast/MonoGame that referenced this pull request Jun 26, 2018
* W10 CoreApplication project Template

* Support for W10 CoreApplication
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

Successfully merging this pull request may close these issues.

5 participants