sharpdx / SharpDX

SharpDX GitHub Repository
http://sharpdx.org
MIT License
1.68k stars 642 forks source link

Function mapping guideline question #1115

Open battlebottle opened 5 years ago

battlebottle commented 5 years ago

Some function Create* functions are mapped with a MyType CreateMyType() pattern, and others are mapped to something like void CreateMyType(out MyType). Are there any general rules to follow in deciding which should be used?

I'm looking at cleaning up and submitting some fixed I've made in MediaFoundation specifically. In mapping-core.xml there's an example for DXGIDeviceManager like this:

<map param="MFCreateDXGIDeviceManager::ppDeviceManager" attribute="out fast"/>

and then there's this for SinkWriter:

<map param="MFCreateSinkWriterFromURL::ppSinkWriter" return="true"/>

Is there any reason in particular these should be different? My intuition would be that creating these rarely created objects, that would generally not be made every frame for example would be better off using the MyType CreateMyType() pattern. I'm interested in any insights here.

xoofx commented 5 years ago

For this particular case, I frankly don't remember. Afair "out fast" is used when having a constructor associated with it. Usually these methods are marked as internal and are only used in the constructor of the class, passing directly the this pointer to the "out fast" argument.

At the beginning, I tried to transform MediaFoundation the same way other APIs were transformed (transforming CreateXXX to new XXX()) but it was so huge and I had a very limited usage of MF that I stopped doing that and I exposed directly the CreateXXX methods.

I would prefer not to introduce breaking changes (because that would require a major bump version, even if SharpDX has been pretty bad at following semver)

battlebottle commented 5 years ago

Thanks @xoofx , that makes things pretty clear.

My understanding then is the best way to proceed is to prefer [CreateXXX to new XXX()] mappings, but if [CreateXXX to void CreateXXX(out XXX)] already exists, then leave it to avoid breaking changes.

How about this then.. I recently submitted a fix for SinkWriter.GetStatistics() which was previously completely broken, but used the (out XXX) pattern before, and I left it like that because I didn't want to change the signature in my fix. Would it be okay if I changed methods that are currently broken or missing to use the new XXX() pattern instead out the (out XXX) pattern?

Also, regarding avoiding breaking changes unless making a major version bump, would it be reasonable for me to maintain a separate branch for any breaking changes I might make in remapping to [new XXX()]? I'm using MediaFoundation for a project at the moment that I'll be maintaining for quite awhile, and I'll most likely choose to clean up the API mappings I use anyways. It would be nice to know I'd have someplace to submit them where they could one day be useful to others.