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

Fix / double-copy for Safari tilemap bug when rendering with delta scrolling #1498

Merged
merged 7 commits into from
Jan 5, 2015

Conversation

pnstickne
Copy link
Contributor

Should fix #1439 (comment) - with automatic selection based on browser detection.

This might fix a safari issue; it also avoids repeatedly setting the
[same] composition operation and saving/loading context states.
- This change uses a secondary canvas and rectangle clearing instead of a
  'copy' composition on the same canvas. This should hopefully address
  the flickering issue in Safari and Safari Mobile
- Memory optimization of delta-scroll shifting.
  - The copy canvas is shared between all TilemapLayers
  - The copy is done in segments to reduce the memory usage of the copy
  canvas. Currently this is a 1/4 ratio.

- Device has the feature (by browser) check to see if bitblt works
  - TilemapLayer will automatically enable the double-copy as needed
  - Device.whenReady can be used to override the device value
- Updated shiftCanvas usage for consistency with renderSetting access
@photonstorm
Copy link
Collaborator

Rather than ua sniffing this should be quite simple to test for by creating a canvas, drawing, performing the copy onto itself and checking the resulting pixel values I'd have thought?

@pnstickne
Copy link
Contributor Author

@photonstorm That is indeed the "correct" approach - and as admitted the current UA approach may miss out on Chrome for iOS, etc. (It should probably use iOS -> force double copy.)

I'm just shy on a device on hammer out the exact details of the failing case.

@photonstorm
Copy link
Collaborator

Sure no worries, I'll add the check in.

The `canvasBitBltShift` detection is updated not to pick up any iOS
browsers. This should eliminate false-positives for Chrome for iOS, eg.

Defaulting to using the double-copy is "safer", even if slightly less
optimal.

(A proper feature check should still be added.)
(Previous changeset not pushed correctly)
@pnstickne
Copy link
Contributor Author

That update (the "real" one) should make it default to a double-copy except when using a known-to-work browser not on iOS.

A proper feature check would be better, but I'm not entirely certain of the exact minimal reproduction case that I can verify/test locally. (Patches to canvasBitBltShift detection welcome!)

While defaulting to a double-copy isn't the most "ideal" it is still able to dramatically benefit from reduced tile redraw (and on a desktop system performs on par with the single bitblt copy); so these last updates err on the side of caution.

photonstorm added a commit that referenced this pull request Jan 5, 2015
Fix / double-copy for Safari tilemap bug when rendering with delta scrolling
@photonstorm photonstorm merged commit ebf650c into phaserjs:dev Jan 5, 2015
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.

None yet

2 participants