brson / rust-sdl

SDL bindings for Rust
MIT License
179 stars 52 forks source link

implemented Clone trait for Surface #136

Open Xirdus opened 10 years ago

Xirdus commented 10 years ago

In this implementation, clone() does deep copy of surface, resulting in independent duplicated pixel data. Alternatively, it could be done using refcounting (SDL already supports that), but it would be counterintuitive when blitting on the copied surface, or using copy-on-write, but this would be much harder to implement.

lifthrasiir commented 10 years ago

Sorry for late reply. I think we should go the same interface as std::rc::Rc, where .clone() means a shallow copy with only refcount increasing and .make_unique() (somehow) means a deep copy. (An implicit copy-on-write is not much used in Rust since it hides the internal cost.) What do you think about this?

Xirdus commented 10 years ago

Well, the problem is, when you blit something on refcounted surface (using SDL's integrated refcounting mechanics), all other shallow copies get overwritten too. We could make some sort of copy-on-write, but this would require additional control structure (we could use SDL_Surface's existing refcount and do copy on blit if rc>1, but this would make us drift away from how SDL handles this value).

I think deep copy makes more sense; std::rc::Rc is a handle with refcount, so "Rc.clone()" technically means "make a deep copy of the handle" - which results in underlying object's shallow copy. I don't think of rust-sdl's surface as a handle, but as a surface itself - so deep copy seemed more natural.