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

FlxCamera: fix object visibility for zoom < 1, closes #292 #2003

Merged
merged 35 commits into from
May 21, 2017
Merged

Conversation

Beeblerox
Copy link
Member

Support for camera zoom values less than 1 for blit and tile render modes

FlxSprites and tilemaps are working.
This work isn't finished and needs documentation
…ous (which is much more perfomance intensive)
Thanks @MSGhero for pointing it
damn, i need to sleep more
scale objects down, not the buffer
@Beeblerox
Copy link
Member Author

@Gama11 could you review this pull request please?

@Gama11
Copy link
Member

Gama11 commented Dec 4, 2016

On Flash, the FlxCamera demo has an issue it didn't have before. If you zoom out all the way to 0.5x and then back to 1x, you get this:

2016-12-04_12-19-53

Note the black borders at the edges.

@Beeblerox
Copy link
Member Author

@Gama11 i'll try to see what's wrong with this

@Beeblerox
Copy link
Member Author

@Gama11 i think i've fixed this issue with FlxCamera demo. At least it works as expected on my machine. Could you check it also?

@starry-abyss
Copy link
Contributor

It looks like @Gama11 's issue was fixed. Also my demo is working too.
But I see that tilemap tearing on Flash in the FlxCamera demo became more frequent at zooms < 1, comparing to dev branch

@Beeblerox
Copy link
Member Author

@starry-abyss FlxCamera demo doesn't use tilemaps. Actually you see gaps between FlxSprites which forms background

But i can't find solution for zoom values less than 1.0 yet
@Beeblerox
Copy link
Member Author

i just can't fix tearing problem on flash target for zoom values less than 1.0. and doubt that this is possible.
Gonna leave it as it is now

@gamedevsam
Copy link
Contributor

Is this change compatible with new opengl renderer?

@Beeblerox
Copy link
Member Author

@gamedevsam i've merged this changes on my working branch recently

@Gama11
Copy link
Member

Gama11 commented Apr 25, 2017

So should this PR be closed?

@Gama11 Gama11 changed the title Camera scaling FlxCamera: fix object visibility for zoom < 1, closes #292 Apr 26, 2017
@Gama11
Copy link
Member

Gama11 commented Apr 26, 2017

@Beeblerox This PR adds some TODO comments, are these still relevant?

@starry-abyss
Copy link
Contributor

What is the reason to not merge it before the OpenFl 4 PR?

/**
* Floored down values of viewOffsetX and viewOffsetY.
*/
private var viewOffsetXFloored:Int = 0;
Copy link
Member

Choose a reason for hiding this comment

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

What's the point of caching these values? Std.int() should be pretty cheap. Also, "floored" implies that Math.floor() was used.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Gama11 ok, i'll remove these variables.

* @param rect rectangle to prepare for rendering
* @return trasformed rectangle with respect to camera's zoom factor
*/
@:noCompletion
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a big fan of @:noCompletion, things still show up in API docs with that (see startQuadBatch() for instance). There's @:dox(hide), but I feel like it's cleaner to just make them private to begin with (and use @:access to access them)?

@Gama11 Gama11 merged commit cbc9269 into dev May 21, 2017
@Gama11 Gama11 deleted the camera-scaling branch May 21, 2017 14:50
Aurel300 pushed a commit to larsiusprime/haxeflixel that referenced this pull request Apr 18, 2018
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.

5 participants