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

Fix crash in XAML design mode #7

Merged
merged 6 commits into from
Sep 2, 2020
Merged

Fix crash in XAML design mode #7

merged 6 commits into from
Sep 2, 2020

Conversation

MikiraSora
Copy link
Contributor

Fix issue : #6
and another bug when control want to generate a opengl buffer in design mode:

Copy link
Owner

@jayhf jayhf left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this issue. I made a few suggestions.

_windowInfo = Utilities.CreateWindowsWindowInfo(
new WindowInteropHelper(Window.GetWindow(this)).Handle);
}
catch
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this necessary? Can't you just add a design mode check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

modified.

mc:Ignorable="d"
d:DesignHeight="300" d:DesignWidth="300">
<Image x:Name="Image" Stretch="Fill" RenderTransformOrigin="0.5,0.5">
<UserControl
Copy link
Owner

Choose a reason for hiding this comment

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

Please don't make unnecessary changes. They just clutter git history

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry it has been formatted automatically by VS plugin XAML Fomatter.

@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<packages>
<package id="OpenTK" version="2.0.0" />
<package id="OpenTK" version="2.0.0" targetFramework="net45" />
Copy link
Owner

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this project has been reference by my other projects and VS modify it automatically. fixed

@@ -305,6 +305,8 @@ public Task RequestRepaint()
return tcs.Task;
}

protected bool IsDesignMode => DesignerProperties.GetIsInDesignMode(this);
Copy link
Owner

Choose a reason for hiding this comment

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

I think inlining this would be more clear

Copy link
Contributor Author

Choose a reason for hiding this comment

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

modified

@MikiraSora MikiraSora requested a review from jayhf October 1, 2019 05:19
@MikiraSora
Copy link
Contributor Author

In fact I notice glReadPixels spend most time on copying buffer into bitmap though CPU. I think it could use WGL_NV_DX_interop and share with DirectX surface.

@jayhf
Copy link
Owner

jayhf commented Oct 2, 2019

The changes look great. I just want to test them before I approve and I probably won't have time until the weekend. You're right that the current CPU copying method isn't efficient and that interop function looks like it could really improve things. I didn't know it existed. I'm not actively developing this library any more, but you're welcome to integrate and PR it if you want.

@MikiraSora
Copy link
Contributor Author

OK I had implemented this opengl extension and release a example.
https://github.com/MikiraSora/OpenTkControl/releases/tag/v0.1.0

I will take a test and format code before raising new PR.

But this is temporary solution because the team of OpenTK dev also want to improve their control library though this extension and WGL_NV_DX_interop2.

Copy link
Owner

@jayhf jayhf left a comment

Choose a reason for hiding this comment

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

Thanks again for the changes. And sorry I forgot about this for so long. Feel free to open a PR that adds WGL_NV_DX_interop2 support if that's still something you're interested in.

@jayhf jayhf merged commit f498205 into jayhf:master Sep 2, 2020
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.

2 participants