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

Implement SDL 0179 - Pixel density and Scale #1401

Conversation

yLeonid
Copy link
Contributor

@yLeonid yLeonid commented Sep 17, 2019

Fixes #1007

Based on the following PM:
#1360 (reference)
Contributors:
@lnafarrateluxoft
#lnafarrateluxoft
@NicoleYarroch
#NicoleYarroch

This PR is ready for review.

Risk

This PR makes minor API changes.

Testing Plan

Unit tests added for:

SDLStreamingVideoLifecycleManagerSpec
SDLVideoStreamingCapabilitySpec
SDLTouchManagerSpec

Functional testing can be done by editing the preferred resolution,
and scale in HMI and testing that the captured view has the proper size and touches are reported
correctly and with the correct coordinates while clicking on the HMI interface.
Also the Automatic Haptic Rect sent to HMI should have the propper size according to the scale value set.

Summary

The main change is related to changing the size of the view captured.
This is done by adding a new scale parameter in the video
streaming capability and by dividing the preferred resolution from the
HMI and the scale.

Also touch coordinates need some changes to match in the HMI with the new resolution, I had to divide the coordinates with the new scale value.

Fixed Travis script to run tests on #Travis CLI

CLA

lnafarrateluxoft and others added 30 commits July 23, 2019 14:53
…nges in the view controller's size to the accepted video resolution divided by the scale.
v6.4 automation moved this from Review in progress to Done Sep 23, 2019
@yLeonid
Copy link
Contributor Author

yLeonid commented Sep 23, 2019

@yLeonid Can you please fetch the changes from develop upstream on your forked repo. Then merge develop into your branch. The develop branch is outdated on this PR and I think doing those steps will fix the issue.

DONE

@yLeonid yLeonid reopened this Sep 23, 2019
v6.4 automation moved this from Done to In progress Sep 23, 2019
fix a typo in string (not an error)

Co-Authored-By: NicoleYarroch <nicole@livio.io>
v6.4 automation moved this from In progress to Review in progress Sep 24, 2019
Copy link
Contributor

@NicoleYarroch NicoleYarroch left a comment

Choose a reason for hiding this comment

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

Left some requests for changes

SmartDeviceLink/SDLCarWindow.m Outdated Show resolved Hide resolved
@yLeonid
Copy link
Contributor Author

yLeonid commented Sep 26, 2019

Travis, all tests passed but 2 in the following files:
SDLMenuManagerSpec
SDLPresentKeyboardOperationSpec

As far as I can see it the 2 are not connected with this subject.

@NicoleYarroch
Copy link
Contributor

@yLeonid Can you let me know when this is ready to review? The following issues still need to be addressed.

  1. The scale value is not being updated in the SDLCarWindow class when the videoStreamingCapability is received from core.
  2. The scale value is not being updated in the SDLFocusableItemLocator class when the videoStreamingCapability is received from core.
  3. Test cases for scaling the haptic rects should be added to SDLHapticManagerSpec

Copy link
Contributor

@NicoleYarroch NicoleYarroch left a comment

Choose a reason for hiding this comment

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

Left some comments requesting changes

SmartDeviceLink/SDLStreamingVideoLifecycleManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/SDLFocusableItemLocator.m Outdated Show resolved Hide resolved
SmartDeviceLink/SDLFocusableItemLocator.m Show resolved Hide resolved
yLeonid added a commit to yLeonid/sdl_ios that referenced this pull request Sep 29, 2019
@yLeonid yLeonid force-pushed the feature/0179_pixel_density_and_scale_v2 branch from c9bad4d to 39dac48 Compare September 29, 2019 16:39
@yLeonid
Copy link
Contributor Author

yLeonid commented Oct 2, 2019

Travis tests are not stable, tests sometimes fail

@NicoleYarroch
Copy link
Contributor

Due to time constraints, we are closing the PR in favor of PR #1420, which was based off of this PR. Thanks for your contributions to this PR.

v6.4 automation moved this from Review in progress to Done Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Accepted SDL Evolution Proposal
Projects
No open projects
v6.4
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants