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

Make setting LogicalPosition and LogicalSize values generic over Into conversions #901

Closed
wants to merge 4 commits into from

Conversation

nvzqz
Copy link
Contributor

@nvzqz nvzqz commented Jun 10, 2019

This allows for doing:

let window: Window = ...;
window.set_outer_position((100, 200));

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

@goddessfreya
Copy link
Contributor

Can this be rebased off the eventsloop-v2 branch?

@nvzqz nvzqz changed the base branch from master to eventloop-2.0 June 10, 2019 03:45
@nvzqz
Copy link
Contributor Author

nvzqz commented Jun 10, 2019

Checks seem to be failing since I changed the base branch on GitHub after I pushed the new changes.

@aloucks
Copy link
Contributor

aloucks commented Jun 10, 2019

It couldn't be merged before you changed the destination branch. Try pushing another commit to trigger the build again.

Copy link
Contributor

@goddessfreya goddessfreya left a comment

Choose a reason for hiding this comment

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

Please make this compile on linux.

@Osspial
Copy link
Contributor

Osspial commented Jun 10, 2019

This PR seems to have a lot of overlap with #895, which does more comprehensive changes to the DPI API.

@nvzqz
Copy link
Contributor Author

nvzqz commented Jun 10, 2019

The intention of this PR is to make it possible to just convert from tuples. Since logical values are what are usually used, it may make sense to implement From<(u32, u32)> and From<(f64, f64)> for Size that constructs a Size::Logical. Likewise for Position.

@Osspial
Copy link
Contributor

Osspial commented Jun 10, 2019

I'm not inclined to implement that conversion, since that implies that there's a "right" way to convert between an untyped coordinate pair and a physical or logical coordinate pair. However, there isn't - one of the reasons for switching to that design is explicitly to de-opinionate Winit in regards to physical/logical coordinate space. Logical and physical coordinate each have their use-cases, and it's been demonstrated that asserting a one-size-fits-all solution will only irritate our users. Enabling (f64, f64) and (u32, u32) conversions is a less harmful echo of that ideology, and I feel it's easier to just not take a stance there at all.

@Osspial
Copy link
Contributor

Osspial commented Jun 12, 2019

With that stated, I'm going to close this PR

@Osspial Osspial closed this Jun 12, 2019
@nvzqz nvzqz deleted the into-conversions branch June 15, 2019 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants