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

#define ImVector #262

Closed
ocornut opened this issue Jul 7, 2015 · 5 comments
Closed

#define ImVector #262

ocornut opened this issue Jul 7, 2015 · 5 comments

Comments

@ocornut
Copy link
Owner

ocornut commented Jul 7, 2015

I am considering removing the ability to #define ImVector to a type that isn't the provided ImGui type.

  • I can start using int instead of size_t which incurs lots of back and forth casting in code to cope with zealous warnings settings. It also makes ImVector<> bigger.
  • I can add helpers to it, have it behave exactly the way I want and generally make the using code more neat. Right now I have to assume that ImVector can be replaced by std::vector so I cannot do those things.
  • I can handle the use of constructor and destructor the way I want, so raw-copiable types that don't need constructions can take a fast path that doesn't cost in debug. Currently ImVector<> omit to call constructors/destructors which can be an annoyance.

I was wondering if anyone it is using the facility to #define ImVector and why?

@emoon
Copy link
Contributor

emoon commented Jul 7, 2015

I say go for it. The way I see it is that ImVector (as many other of the Im types) is a implementation detail that very few needs to care about.

@extrawurst
Copy link
Contributor

+1 - I agree with Daniel!

On Tue, Jul 7, 2015 at 8:50 AM, Daniel Collin [email protected]
wrote:

I say go for it. The way I see it is that ImVector (as many other of the
Im types) is a implementation detail that very few needs to care about.


Reply to this email directly or view it on GitHub
#262 (comment).

@Pagghiu
Copy link
Contributor

Pagghiu commented Jul 7, 2015

+1 - I agree

If one needs an automatic conversion from their vector struct/class to ImVector could always define operator ImVector() and be happy...

ocornut added a commit that referenced this issue Jul 7, 2015
ocornut added a commit that referenced this issue Jul 7, 2015
@ocornut
Copy link
Owner Author

ocornut commented Jul 7, 2015

OK done that, changed the size_t to int which allowed to remove many of the casting in the code.
Haven't added new functions to ImVector<> but will do as required.

I tested some combinations of visual/clang 32/64 but there's probably new warnings to be found with some compilers.

@ocornut
Copy link
Owner Author

ocornut commented Jul 7, 2015

Very happy with this change. Removed a bunch of cast. Using the fields directly, it's shorter to write, easier to step into and much faster in debug builds where inline calls are recorded (measured a 10% difference in my benchmarks).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants