cocos2d / cocos2d-objc

Cocos2d for iOS and OS X, built using Objective-C
http://www.cocos2d-objc.org
Other
4.07k stars 1.16k forks source link

CCColor class #407

Closed vlidholt closed 10 years ago

vlidholt commented 10 years ago

The problem: There are many different color types in Cocos2d and it's hard for a beginner to immediately grasp the differences and when to use which one. Providing a single color class for the public interface would simply this. Another advantage is that the actual implementation can change (maybe it would be more efficient in some case to use ccc4f instead of ccc3b, now this cannot be changed without breaking the API). Sprite Kit uses this approach with the SKColor class.

CCColor would have an interface similar to this:

// Constructors
CCColor* myColor = [CCColor colorWithRed: 1.0 green: 1.0 blue: 1.0];
CCColor* myColor = [CCColor colorWithRed: 1.0 green: 1.0 blue: 1.0 alpha: 1.0];

// Accessing color components
float red = myColor.red;

// Getting data structures for use in GL code
ccColor3B glColorStruct = myColor.ccColor3b;

Implementation: This could be implemented the same way as in SK, by using the native color classes and adding categories to get a unified interface and the extra methods.

#if TARGET_OS_IPHONE
#define CCColor UIColor
#else
#define CCColor NSColor
#endif
Mazyod commented 10 years ago

@vlidholt We ran into unnecessary problems with our games when we #defined NSColor and UIColor. They are pretty similar, but not exactly the same, which may lead to issues. Some frameworks use the #define for simplifying the internal framework, but not expose it to the users, which is a wise approach, IMHO.

Making a CCColor wrapper around NSColor and UIColor is worth it, IMHO.

slembcke commented 10 years ago

Oh, I had forgotten about this. A CCColor type that aliases UIColor/NSColor was originally my thought too. It's dirt simple and interesting that SK does the same thing. Reusing existing OS types, even simple ones is never a bad idea.

I'd also be pretty happy with treating it as another value type like CGPoint. Just a float 4 vector and a small handful of C functions.

@Mazyod Was the issue just that you were using methods that were defined on one but not the other?

An easy/inexpensive way to hide the differences could be to implement it like a class cluster. Make the CCColor methods return UIColor or NSColor objects depending on platform.

vlidholt commented 10 years ago

NSColor can be a bit painful to use from my experience. It often tries to do to much (in terms of color synch and what not and can make results differ on different displays). If we go for the #define approach it would be a good idea to add the UIColor constructors in a category so users can use the same interface.

Birkemose commented 10 years ago

I vote we roll our own colour type, also because 64 bit will result in floats becoming doubles, which doesn't work well with OpenGL.

vlidholt commented 10 years ago

For OpenGL purposes UIColor seems to be a good, clean, option. The problem with NSColor is that it implements the UIColor compatibility methods in 10.9, which means that using NSColor out of the box wouldn't work with older versions os Mac OS X. After some more research, my suggestion is that we use slightly different implementations on Mac and iOS:

iOS: Simply define CCColor as UIColor Mac: Implement CCColor as a class with the same interface as UIColor, any conversions between color spaces (RGB/HSB) can be done using the NSColor class.

@Birkemose using a class means that we will need to care less about how the color is stored and that we can change this easily if needed in the future. The class will not be used directly in the OpenGL code, but serve as a common interface for the public API. You would still have all options for using whatever color types are best for a specific OpenGL case. (E.g. use myColor.ccColor3B in OpenGL.) The best option for storing the actual r,g,b values in the class seems to be CGFloat (this is what both UIColor and NSColor uses).

Birkemose commented 10 years ago

As far as I understand it, UIColor will be doubles on 64 bit targets, while GLfloat will continue to be floats. So either we will have to manually assign each value, or we will have to start using doubles with openGL when in 64. I fear both things will result in a significant performance drop compared to running strictly floats.

vlidholt commented 10 years ago

@Birkemose, what is used in the CCColor class must not necessarily be the same as what is used in OpenGL. One of the basic principles of OO development is data hiding, by providing a common interface we can change the implementation without affecting backwards compatibility. Say today 32 bit floats are the way to go, but in a year the most efficient solution is 64 bit. Having a class handling this will ensure we do not have to change the interface.

slembcke commented 10 years ago

I forgot about this thread from the other day.

Storing colors in the objects as 64 bit floats probably wouldn't even make a measurable performance impact since you can convert to something more suitable (like float4) when assigning to a property. Sprites default to opaque white, and changing it is a rare thing to do (fades, tinting, etc). I would be surprised if the assignment cost even makes a measurable difference.

It's also worth noting that CGPoint and CGFloat use 64 bit doubles on the 64 bit ABI too.

slembcke commented 10 years ago

Some of the color changes from 352e11a7cd94b604e0a93340b12447f1332fc47a seem to have broken tilemaps (probably anything atlas based maybe?). They show up as black. Running it in the GL debugger, the values in the VBO are 1 (byte not float). It's probably an easy fix, but can't figure out where yet.

edit: Getting closer, _displayedOpacity for the tile sprites is 1. Regular sprites work fine though. edit2: We need to audit all calls to setOpacity:. Since floats can be implicitly cast to bytes without a warning. Found a couple instances of that so far. edit3: I think I found them all, and there were only two incorrect calls. That was what was causing the problem I had, so it's fixed.

vlidholt commented 10 years ago

Thanks for the fixes!

Birkemose commented 10 years ago

About performance, I agree that it might not make a huge difference at the end of the day. I guess my main gripe is, that we in less than a year, will have an API running different float sizes on most modern devices. I have added a memo to myself, about looking into switching openGL to 64 bit, on 64 bit targets. That would in my world make a lot of sense.

billhollings commented 10 years ago

@Birkemose

Just to be clear...what do you mean when you mention "switching OpenGL to 64 bit"?

Are you expecting the OpenGL ES declarations for GLfloat, etc., to become 64 bit in the near future? If so, where are you getting that from?

OpenGL ES 3.0 does introduce a GLint64 type, but it is not used at all in the API. And GLdouble does not exist for OGLES 3.0.

And even OpenGL on OSX runs with GLfloat and GLclampf as common 32 bit API's. Most OpenGL/OSX calls have both GLfloat and GLdouble versions.

...Bill

Birkemose commented 10 years ago

Yeah, I was meaning changing GLfloats's to GLdouble's, but as you say, that is not supported on ES2. Not that I am surprised, as 32 bit seems to be more than sufficient for rendering. Basically means, that I will never be able to create a buffer of CGPoint's, use it for custom rendering, and then memcpy it to my VBO.

slembcke commented 10 years ago

Unfortunately no, and you shouldn't really do that on 64 bit Macs right now either. My (2011?) iMac has a pretty beefy Radeon GPU in it and it's really slow to pass a buffer full of doubles to it. I assume it's because the shader processors can't even handle them at all, and need to convert when loading them or something expensive.

In the debug renderer for the desktop Chipmunk demo app, I actually convert to float on the CPU since it's an order of magnitude faster than letting the GPU do it. I have no idea if newer desktop GPUs can handle doubles, but it's almost certainly not going to change for mobile GPUs for many years.

billhollings commented 10 years ago

Agreed.

IMHO...if CGPoint is becoming platform-dependent, that provides reason to create ccVertex {GLfloat, GLfloat} (or ccPoint {GLfloat, GLfloat} if you prefer), that can be used for vertex content (vertex position and tex coords).

This is exactly the reason why GLfloat, etc were created by the OpenGL folks in the first place.

BTW...cocos3d has CC3Vertex {GLfloat, GLfloat, GLfloat}, for the same reason.

Regarding Scott's last thought, doubles are supported on the desktop, but the desktop API provides calls for both doubles and floats...so you're never forced to use doubles.

slembcke commented 10 years ago

Continuing to use CGPoints in the main API is fine I think. The only places where it's really going to matter is when you need to create a buffer of 32 bit floats specifically, and that really is only going to happen when writing GL code in the renderer.

IMO, you should always keep your GL and app types separate anyway. Desktop CPUs are often faster when using doubles since that is how they are handled internally anyway. Doubles used to be faster on x86 and PPC for Chipmunk, now double/float performance is about the same.

I don't know if the new Apple CPU can handle doubles as efficiently as floats, but it's so much faster that will still almost certainly be a large net gain in performance anyway. Apple's ABI choices affect a lot more than just Cocos2D.

Regarding Scott's last thought, doubles are supported on the desktop, but the desktop API provides calls for both doubles and floats...so you're never forced to use doubles.

and shouldn't use them! ;) There are a small number of desktop GL features that have basically never had good hardware support like antialiased primitives, certain data types, noise functions in GLSL, etc. While you could get away with using them on certain hardware, on others it would have severe performance penalties or may even incur a software fallback.

slembcke commented 10 years ago

Ok! Embarrassing. I think I've been similarly confused in this thread because I skimmed the code too quickly. I thought that the CCColor class was using UIColor/NSColor like was discussed earlier, and I thought that is where some of the issues were coming from. (Woo confirmation bias)

Since we've written our own class from scratch already, I think there is little reason to use CGFloat over regular floats. Nobody needs that much HDR in games. O_o I think Andy is making some of those changes in a patch now.

andykorth commented 10 years ago

I'll make that change as part of issue https://github.com/cocos2d/cocos2d-iphone/issues/430

This will be nice because we can pass the colors directly to openGL.

[_shaderProgram setUniformLocation:_uniformColor with4fv:&_displayColor count:1];
vlidholt commented 10 years ago

I first made extensions to UIColor and reimplemented UIColor on Mac. However, the UIColor uses different color spaces that, for the purposes of Cocos2d, makes things unnecessarily complicated (and possibly inefficient). Therefore I moved to an own implementation of CCColor, independent of UIColor and NSColor (you can still use the native color types create a CCColor or get it as a read only property so it's easy to integrate with other Cocoa code).

slembcke commented 10 years ago

Yeah, I kind of figured that's why it ended up with CGFloats.

vlidholt commented 10 years ago

Like the new updates! The only thing that I think isn't super clear is lerpTo: maybe it would be nice to add a few extra characters to make it more clear. At least I hadn't heard the term before.

andykorth commented 10 years ago

Ah, sure. Unity uses that phrase a lot, but I think it is sort of uncommon.

vlidholt commented 10 years ago

I think we can close this one now!