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

Added Win32 Monitor Configuration API and demo #332

Closed
wants to merge 6 commits into from

Conversation

msteiger
Copy link
Contributor

This adds the Windows Monitor Configuration API methods and structures. I tested the port with a small monitor demo which is also part of this PR. It enumerates all monitors and displays their caps.

@@ -9,6 +9,7 @@ Features
--------
* Updated AIX natives and build - [@twall](https://github.com/twall).
* [#290](https://github.com/twall/jna/pull/290): Improved the stacktrace for the exceptions thrown by `com.sun.jna.Structure` - [@ebourg](https://github.com/ebourg).
* Added Win32 Monitor Configuration API
Copy link
Member

Choose a reason for hiding this comment

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

Please follow the syntax of other CHANGELOG entries, copy from above. Thx.

@dblock
Copy link
Member

dblock commented May 21, 2014

This is good work. However, all these new functions need tests to be merged. A demo is not good enough :)

Minor: I am not convinced FlagEnum and EnumConverter should live where they are, I am worried that .platform becomes a repository for utility functions. Not sure where it should go. The class itself needs its own tests.

Something with tabs/spaces that looks off, there's a lot of them and things are aligned the same way. Maybe run some automatic formatter on this? Would be nice to fix.

@msteiger
Copy link
Contributor Author

Thanks - I updated the PR:

  • Fixed typo
  • Updated CHANGES.MD
  • Fixed whitespace/tabs
  • Added tests in User32Test
  • Created Dxva2Test test case - also tests EnumConverter and EnumUtils (not sure how to test them independently)

return 0; // stop
}
}, new LPARAM(0));
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we assert something in this test? For example that you have at least one monitor? Then, each one of the Dxva functions should also be an actual test. For example, GetMonitorCapabilities returns some values in temps and caps, assert that they are not 0 for example.

@msteiger
Copy link
Contributor Author

I split the tests into smaller ones and added asserts where possible.
Please note that the test case will succeed only if at least one monitor is attached - i.e. not on a build server.

@dblock
Copy link
Member

dblock commented May 24, 2014

Good work. Merged via bf870bb (you might want to squash your own commits, I try to keep things tidier on merge).

@dblock dblock closed this May 24, 2014
@msteiger
Copy link
Contributor Author

Thanks! I would throw out my master branch and create a new from yours. Should have the same effect, right?

@dblock
Copy link
Member

dblock commented May 26, 2014

You should definitely reset your master to the upstream master, first. I usually do this by doing a git reset --hard a few commits back, then git pull upstream master, then a force push of git push origin master -f. (If you have changes on master that need to be saved first, git checkout -b some-new-branch, then git checkout master first).

I always work on branches. When I make small changes, I git commit --amend them, sometimes force push. No fear :)

@wolftobias
Copy link
Contributor

PLease let me understand the background of your mail. I`m I using a outdated code base? I just want to avoid overwriting my changes.
 

Gesendet: Montag, 26. Mai 2014 um 12:58 UhrVon: "Daniel Doubrovkine (dB.) @dblockdotorg" [email protected]: twall/jna [email protected]: Re: [jna] Added Win32 Monitor Configuration API and demo (#332)

You should definitely reset your master to the upstream master, first. I usually do this by doing a git reset --hard a few commits back, then git pull upstream master, then a force push of git push origin master -f. (If you have changes on master that need to be saved first, git checkout -b some-new-branch, then git checkout master first).

I always work on branches. When I make small changes, I git commit --amend them, sometimes force push. No fear :)


Reply to this email directly or view it on GitHub.

@wolftobias
Copy link
Contributor

@dblock Are you sure you are meaning me? I`ve not commited this commit request. Please let me know if there was a mistake.

@dblock
Copy link
Member

dblock commented May 27, 2014

I meant this for @msteiger.

@msteiger
Copy link
Contributor Author

I think I figured it out - thanks!

@dblock
Copy link
Member

dblock commented Jun 25, 2014

I'm seeing this on my box now after this PR was merged:

    [javac] Compiling 24 source files to c:\Users\dblock\source\jna\twall\contrib\platform\build\classes
    [javac] warning: [options] bootstrap class path not set in conjunction with -source 1.5
    [javac] c:\Users\dblock\source\jna\twall\contrib\platform\src\com\sun\jna\platform\win32\Dxva2.java:51: error: diamo
nd operator is not supported in -source 1.5
    [javac]                         addTypeConverter(MC_POSITION_TYPE.class, new EnumConverter<>(MC_POSITION_TYPE.class)
);
    [javac]                                                                                    ^
    [javac]   (use -source 7 or higher to enable diamond operator)
    [javac] 1 error
    [javac] 1 warning

@dblock
Copy link
Member

dblock commented Jun 25, 2014

Fixed in de798fd.

@dblock
Copy link
Member

dblock commented Jun 27, 2014

I added 1c7e616 which will skip the tests when there's no physical monitor attached, @msteiger check the code please I am not sure this is the best way. Feel free to PR updates.

mstyura pushed a commit to mstyura/jna that referenced this pull request Sep 9, 2024
Motivation:

jabba is not updated anymore, let's use sdkman for JDK installation

Modifications:

- Switch to sdkman
- Update to latest JDK8 release

Result:

Be able to use latest JDK version again
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.

3 participants