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 misalignment of multi-touch emu feedback #3506

Merged
merged 3 commits into from Aug 1, 2015

Conversation

Projects
None yet
2 participants
@cpbotha
Contributor

cpbotha commented Jul 15, 2015

Visual feedback of multi-touch emulation first touch was being placed
at coordinates uncorrected for density != 1. Fixed by factoring out
and using same scaling as for WindowBase.on_motion().

Fix misalignment of multi-touch emu feedback
Visual feedback of multi-touch emulation first touch was being placed
at coordinates uncorrected for density != 1. Fixed by factoring out
and using same scaling as for WindowBase.on_motion().
Show outdated Hide outdated kivy/core/window/__init__.py
effective_size = AliasProperty(_get_effective_size, None)
'''On density=1 and non-ios displays, return system_size, else
return scaled / rotated size.

This comment has been minimized.

@akshayaurora

akshayaurora Jul 16, 2015

Member

.. versionadded: 1.9.1

I understand the need for the function to get DRY but the property adds another level of complexity that the user does not need to know about, this is and should be hidden from the user.

Is there any particular reason for this property? We already have size and system_size to differentiate between the window size and the size of the content.

@akshayaurora

akshayaurora Jul 16, 2015

Member

.. versionadded: 1.9.1

I understand the need for the function to get DRY but the property adds another level of complexity that the user does not need to know about, this is and should be hidden from the user.

Is there any particular reason for this property? We already have size and system_size to differentiate between the window size and the size of the content.

This comment has been minimized.

@cpbotha

cpbotha Jul 16, 2015

Contributor

My reasons for doing it this way was indeed to keep it DRY (having two different places checking the same logic is a bit fragile), and to follow the lead of the other properties.

Would you prefer that we rather just remove the property, and then rename the function to "get_effective_size()" (without underscore) and use that? (or are we ok with calling a private function from MouseMotionEvent?)

@cpbotha

cpbotha Jul 16, 2015

Contributor

My reasons for doing it this way was indeed to keep it DRY (having two different places checking the same logic is a bit fragile), and to follow the lead of the other properties.

Would you prefer that we rather just remove the property, and then rename the function to "get_effective_size()" (without underscore) and use that? (or are we ok with calling a private function from MouseMotionEvent?)

This comment has been minimized.

@akshayaurora

akshayaurora Jul 17, 2015

Member

This function would be used internally only and thus it makes sense to keep
it private, i.e have it start using _.

On Thu, Jul 16, 2015 at 4:43 PM, Charl P. Botha notifications@github.com
wrote:

In kivy/core/window/init.py
#3506 (comment):

@@ -528,6 +528,21 @@ def _get_system_size(self):
'''Real size of the window ignoring rotation.
'''

  • def _get_effective_size(self):
  •    w, h = self.system_size
    
  •    if platform == 'ios' or self._density != 1:
    
  •        w, h = self.size
    
  •    return w,h
    
  • effective_size = AliasProperty(_get_effective_size, None)
  • '''On density=1 and non-ios displays, return system_size, else
  • return scaled / rotated size.

My reasons for doing it this way was indeed to keep it DRY (having two
different places checking the same logic is a bit fragile), and to follow
the lead of the other properties.

Would you prefer that we rather just remove the property, and then rename
the function to "get_effective_size()" (without underscore) and use that?


Reply to this email directly or view it on GitHub
https://github.com/kivy/kivy/pull/3506/files#r34777003.

@akshayaurora

akshayaurora Jul 17, 2015

Member

This function would be used internally only and thus it makes sense to keep
it private, i.e have it start using _.

On Thu, Jul 16, 2015 at 4:43 PM, Charl P. Botha notifications@github.com
wrote:

In kivy/core/window/init.py
#3506 (comment):

@@ -528,6 +528,21 @@ def _get_system_size(self):
'''Real size of the window ignoring rotation.
'''

  • def _get_effective_size(self):
  •    w, h = self.system_size
    
  •    if platform == 'ios' or self._density != 1:
    
  •        w, h = self.size
    
  •    return w,h
    
  • effective_size = AliasProperty(_get_effective_size, None)
  • '''On density=1 and non-ios displays, return system_size, else
  • return scaled / rotated size.

My reasons for doing it this way was indeed to keep it DRY (having two
different places checking the same logic is a bit fragile), and to follow
the lead of the other properties.

Would you prefer that we rather just remove the property, and then rename
the function to "get_effective_size()" (without underscore) and use that?


Reply to this email directly or view it on GitHub
https://github.com/kivy/kivy/pull/3506/files#r34777003.

Show outdated Hide outdated kivy/core/window/__init__.py
w, h = self.system_size
if platform == 'ios' or self._density != 1:
w, h = self.size
w,h = self.effective_size

This comment has been minimized.

@akshayaurora

akshayaurora Jul 16, 2015

Member
w,  h 

You need the pep8hook.
run

make hook

from the kivy root dir.

@akshayaurora

akshayaurora Jul 16, 2015

Member
w,  h 

You need the pep8hook.
run

make hook

from the kivy root dir.

This comment has been minimized.

@akshayaurora

akshayaurora Jul 16, 2015

Member

Let's use the function and get rid of the property.

@akshayaurora

akshayaurora Jul 16, 2015

Member

Let's use the function and get rid of the property.

@akshayaurora akshayaurora added this to the 1.9.1 milestone Jul 17, 2015

@cpbotha

This comment has been minimized.

Show comment
Hide comment
@cpbotha

cpbotha Jul 25, 2015

Contributor

bump Could you take another look at this?

(I've just noticed that there's similar buggy behaviour when handling a drop event on the window. would like to have this PR merged before I try and fix the next related issue.)

@akshayaurora

Contributor

cpbotha commented Jul 25, 2015

bump Could you take another look at this?

(I've just noticed that there's similar buggy behaviour when handling a drop event on the window. would like to have this PR merged before I try and fix the next related issue.)

@akshayaurora

Show outdated Hide outdated kivy/core/window/__init__.py
@@ -528,6 +528,18 @@ def _get_system_size(self):
'''Real size of the window ignoring rotation.
'''
def get_effective_size(self):

This comment has been minimized.

@akshayaurora

akshayaurora Jul 29, 2015

Member

There is no need for this to be exposed to the user, this is used internally only and thus should start with a _ _get_effective_size.

@akshayaurora

akshayaurora Jul 29, 2015

Member

There is no need for this to be exposed to the user, this is used internally only and thus should start with a _ _get_effective_size.

@cpbotha

This comment has been minimized.

Show comment
Hide comment
@cpbotha

cpbotha Aug 1, 2015

Contributor

Hi there @akshayaurora !

Have renamed method with underscore. The travis checks that have failed are in kivy/config.py (lines too long) -- but my commit has not touched those files.

Please let me know if you would prefer me squashing the three commits together, ok?

Contributor

cpbotha commented Aug 1, 2015

Hi there @akshayaurora !

Have renamed method with underscore. The travis checks that have failed are in kivy/config.py (lines too long) -- but my commit has not touched those files.

Please let me know if you would prefer me squashing the three commits together, ok?

akshayaurora added a commit that referenced this pull request Aug 1, 2015

Merge pull request #3506 from cpbotha/fix_issue_3366
Fix misalignment of multi-touch emu feedback

@akshayaurora akshayaurora merged commit 4c9e591 into kivy:master Aug 1, 2015

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
@akshayaurora

This comment has been minimized.

Show comment
Hide comment
@akshayaurora

akshayaurora Aug 1, 2015

Member

@cpbotha nice job!

Member

akshayaurora commented Aug 1, 2015

@cpbotha nice job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment