Addresses Issue: Unable to "teleport" to connected screen

egirsova commented 9 years ago

This pull request resolves issue #15, as well as similar issues discussed in the forums (for example: http://abyssoft.com/software/teleport/forums/index.php?topic=42928.0).

Brief Issue Explanation

Application appears to have been configured (i.e. the remote screen has been detected, the user moves the remote screen view near the edge of the local screen view in the layout area, and the actual local screen edge becomes highlighted with red, seemingly indicating that the two screens are connected), but no "teleportation" actually happens when the user touches the corresponding edge of the screen with their mouse.

Cause

I found that this happens because the remote host is unable to actually configure properly, because it throws the exception:

ImageIO: CGImageDestinationFinalize image destination must have at least one image
CGImageDestinationFinalize failed for output type 'public.tiff'

This exception is thrown in the method backgroundImageData in TPHost.m during the line:

NSData * backgroundImageData = [backgroundImage TIFFRepresentationUsingCompression:NSTIFFCompressionLZW factor:1.0];

In this case, the NSImage, while not nil, has size {0,0}. This is because the file path that was used to create this NSImage object is pointing to a file that does not actually exist (the file path is calculated in _desktopPicturePathForScreen in TPLocalHost.m). It is also clear that the screens are not connected properly because the "options" preference never appears in the layout of the screen view.

Issue Occurrences

I found that this issue occurs in two main cases:

The user had just upgraded their operating system and had not yet changed their desktop background manually (so the current desktop image points to "nothing")
The user has their desktop background set to changing automatically, so it doesn't get the correct file path.
NOTE: From reading the forum posts, I found that multiple users were actually able to get Teleport to work by simply changing their desktop background or switching off the "set desktop background automatically" preference.
Fix/Changes:
First check if a file actually exists at

[[NSWorkspace sharedWorkspace] desktopImageURLForScreen:screen]

(which is the desktop image path of the current screen) and, if it doesn't, return nil. This way, Teleport would have to generate an image based on the default OS desktop background provided with the app, rather than on the current desktop image.

Added default desktop images for the most recent operating systems, to make sure that there will be no case where a desktop image is unavailable to the user.
Additional Comments
When reading the logs, I also found the output error "Can't set TXT record data" that occurred when the app was opened and it's "Share this Mac" preference was set to FALSE. This caused a notification to be sent to call the method updateTXTRecordOfPublishService in TPBonjourController.m, which occurred before _publishService was actually initialized. So, I fixed this issue by first checking whether or not _publishService already exists before running the rest of the method.

dmitshur commented 9 years ago

Hi @egirsova,

(I'm new to this repository and the app; I've used it for 2 days, reported one issue and contributed no code so far, so take my comment FWIW.)

I wanted to say I really appreciated the detail and thoroughness you've put into this PR description. I started off unfamiliar with the context and having that description made it very easy to understand what's going on.

I've reviewed the code and it looks very clean. All changes made sense given the findings you've outlined. I wasn't able to spot anything wrong, so it LGTM.

Thanks for making the fix!

egirsova commented 9 years ago

Hi @shurcooL, Thanks for taking the time to read the PR description, as well as for your comment! I appreciate it.

abyssoft commented 9 years ago

Thanks for these changes, they look good!

chromixsea commented 9 years ago

is it possible to get a new build / release for these changes?

I'd like to give it a try but I'm not interested in getting into the whole build thing at this point...

abyssoft commented 9 years ago

Alright, I'll make a dot release as soon as I get the time.

johndbritton / teleport