libtcod / python-tcod

A high-performance Python port of libtcod. Includes the libtcodpy module for backwards compatibility with older projects.
BSD 2-Clause "Simplified" License
410 stars 36 forks source link

add put_char_ex method to console #84

Closed psizek closed 5 years ago

psizek commented 5 years ago

put_char is in the tcod.console methods, but put_char_ex is not - this adds the latter method.

HexDecimal commented 5 years ago

You forgot to add commas after the fg and bg parameters, but I probably won't be able to merge this without some discussion.

A put_char_ex method is not ideal due to the performance issues caused by this kind of function. High numbers of Python function calls and many Python to C type conversions can be a significant source of slowdown.

put_char exists because it depended on a side-effect of some other functions, but "console defaults" are being phased out.

The ideal would be to have a modified version of the Console.tiles attribute which doesn't have any alpha channels. Something like that could be used in place of a function like put_char_ex but would also be vectorizable.

psizek commented 5 years ago

I think I see what you mean.

Question: What's the reason to keep the put_chars stuff separate from the Console.tiles attribute/have it be vectorizable? I.e., my immediate instinct/future pull request would be to make put_char_ex set the tiles directly and default in an opaque alpha channel. Debugging, it looks like this is what put_char_ex basically ends up doing right now, but I could be missing something.

I can imagine a world in which you might have a chars array and a tiles array so you can track the chars separately from the "background" tiles and overlay the two when you blit? If your background tiles wouldn't change most of the time, I could see this being useful so you wouldn't need to rewrite them to the console each time.

HexDecimal commented 5 years ago

Question: What's the reason to keep the put_chars stuff separate from the Console.tiles attribute/have it be vectorizable? I.e., my immediate instinct/future pull request would be to make put_char_ex set the tiles directly and default in an opaque alpha channel.

The reason not to add put_char_ex is that it can hide vectorizable NumPy code:

for i in ...:
    for j in ...:
        func(array, i, j, v)  # Can't be easily vectorized because of the i,j parameters.
for i in ...:
    for j in ...:
        array[i, j] = v  # A more obviously vectorizable loop.
array[:, :] = v  # Much faster than the above examples.

Debugging, it looks like this is what put_char_ex basically ends up doing right now, but I could be missing something.

You're correct. put_char_ex ignores the console defaults and just sets the values directly.

The main problem is that Console.tiles has an often unnecessary alpha channel which you have to mess with almost every time you assign to it. I've worked on this but I've been stuck on silly things like what to name the attribute. I'll probably end up calling it Console.tiles2.

I can imagine a world in which you might have a chars array and a tiles array so you can track the chars separately from the "background" tiles and overlay the two when you blit? If your background tiles wouldn't change most of the time, I could see this being useful so you wouldn't need to rewrite them to the console each time.

If you're talking about just colors vs glyphs then this already exists both with how Console.tiles works as a structured array (Console.tiles["ch"] returns just the character array) and the redundant Console.ch, Console.fg, and Console.bg array attributes which are currently derived from Console.tiles at the moment.

A lot of the newest Console methods won't set the colors unless given a specific color to use.