Allegro.cc - Online Community

Allegro.cc Forums » Allegro Development » al_get_window_position

Credits go to Peter Wang for helping out!
This thread is locked; no one can reply to it. rss feed Print
 1   2 
al_get_window_position
Matthew Leverton
Supreme Loser
January 1999
avatar

al_get_window_position is returning (0,0) on Ubuntu. The Allegro code looks like:

   ALLEGRO_DISPLAY_XGLX *glx = (ALLEGRO_DISPLAY_XGLX *)display;
   /* We could also query the X11 server, but it just would take longer, and
    * would not be synchronized to our events. The latter can be an advantage
    * or disadvantage.
    */
   *x = glx->x;
   *y = glx->y;

When do glx->x and glx->y get updated? Even the following fails (sets x and y to 0):

int sx, sy;
ALLEGRO_DISPLAY *display = al_get_current_display();
al_set_window_position(display, 100, 100);
al_get_window_position(display, &sx, &sy);

Also, why do al_get_window_position and al_set_window_position take a ALLEGRO_DISPLAY parameter when none of the other functions do?

Elias
Member #358
May 2000

When do glx->x and glx->y get updated? Even the following fails (sets x and y to 0):

It's supposed to be updated in _al_display_xglx_configure. Basically in al_set_window_position we send a request to the X11 server to move the window but immediately return. That request travels to the server at some later time and the server creates a reply and then much later when X11 decides to sync to the client a "Configure" event supposedly arrives.

The solution is probably to update our internal position (glx->x,y) immediately - I can't see any problem with that.

Quote:

Also, why do al_get_window_position and al_set_window_position take a ALLEGRO_DISPLAY parameter when none of the other functions do?

I think we should remove that parameter. We never got around to do that long planned "API consistency check" we originally decided to do before the A5 release. So some of the function names and parameter orders aren't consistent yet.

--
"Either help out or stop whining" - Evert

Kitty Cat
Member #2,815
October 2002
avatar

Elias said:

The solution is probably to update our internal position (glx->x,y) immediately - I can't see any problem with that.

Might be a problem if the request fails for some reason. Perhaps a better solution is to specify that al_set_window_position will work asyncronously, and some event will be posted when the actual move occurs? Or make al_set_window_position block until the move succeeds or fails.

--
"Do not meddle in the affairs of cats, for they are subtle and will pee on your computer." -- Bruce Graham

Peter Wang
Member #23
April 2000

This is slightly tangential, and I haven't thought this through fully, but..

The relationship between the current target bitmap and the current display can be rather confusing; setting one will sometimes imply the other (can't remember without looking up the docs). Could we do with only having the current target bitmap? If the target bitmap is the front/back-buffer of a display, then that display is the "current" display. Otherwise the thread has no current display.

Elias
Member #358
May 2000

The relationship between the current target bitmap and the current display can be rather confusing; setting one will sometimes imply the other (can't remember without looking up the docs). Could we do with only having the current target bitmap? If the target bitmap is the front/back-buffer of a display, then that display is the "current" display. Otherwise the thread has no current display.

I think it was like that already at one point. And I wouldn't mind.

I assume technically it just means replacing ALLEGRO_DISPLAY with ALLEGRO_BITMAP throughout the API. ALLEGRO_DISPLAY would only be an internal type then. And al_set_target_bitmap would set the current display to NULL except when the passed bitmap is the backbuffer of a display, in which case it would set that as the current display. Should be a quite simple change. A normal application like below would need no change at all:

al_create_display(1000, 1000);
al_draw_line(0, 0, 1000, 1000, 1);
al_flip_display();

Only where you use al_set_target_bitmap() right now and don't restore it to the backbuffer before calling a function operating on the current display a change is required in user code (for a single-display application).

There's much bigger breaking changes than that in 4.9.21 already :P

--
"Either help out or stop whining" - Evert

Evert
Member #794
November 2000
avatar

Elias said:

I assume technically it just means replacing ALLEGRO_DISPLAY with ALLEGRO_BITMAP throughout the API.

Huh?
It doesn't make sense to me to set the icon for a bitmap, the title, to move a bitmap or to request a full-screen bitmap on the second monitor.
At least to my mind, bitmaps and displays are logcially distinct and not interchangeable. I don't think that Peter's suggestion (which makes sense to me) changes that in any way.

Elias
Member #358
May 2000

With Peter's suggestion you could not change the window title while an offscreen bitmap or memory bitmap is the drawing target. If we keep ALLEGRO_DISPLAY as separate type that would make no sense at all. The idea behind the suggestion was to to make things less confusing not more, so I'd say it implies getting rid of ALLEGRO_DISPLAY. If you can't set or query it nor pass to any functions, why confuse the user with exposing the type?

--
"Either help out or stop whining" - Evert

Thomas Fjellstrom
Member #476
June 2000
avatar

I think theres really only two options, keep the current setup and make sure things work properly. Or change it so display api functions take ALLEGRO_DISPLAYs, and get rid of the TLS state for the current display.

--
Thomas Fjellstrom - [website] - [email] - [Allegro Wiki] - [Allegro TODO]
"If you can't think of a better solution, don't try to make a better solution." -- weapon_S
"The less evidence we have for what we believe is certain, the more violently we defend beliefs against those who don't agree" -- https://twitter.com/neiltyson/status/592870205409353730

SiegeLord
Member #7,827
October 2006
avatar

I think this change unnecessarily conflates display specific functionality with drawing, which is display independent. It also creates even more magical types than there are now...

I for one would prefer not to have the al_get_backbuffer functions at all, since the bitmaps returned by those functions are very limited (see the docs) and have al_set_target_bitmap(0) make the target bitmap be the backbuffer again (some special casing/functioning can be done for the frontbuffer). For using backbuffer as a source bitmap, I'd have a al_copy_from_backbuffer or something, since that's the only thing we can do right now anyway (everything else falls back to software routines).

As for where the current bitmap belongs... it doesn't really matter I suppose, both making it thread local (as it is now) and display local (as it actually is) are both workable solutions, since in the end, you can't use multiple displays in a single thread anyway.

"For in much wisdom is much grief: and he that increases knowledge increases sorrow."-Ecclesiastes 1:18
[SiegeLord's Abode][Codes]:[DAllegro5]:[RustAllegro]

Matthew Leverton
Supreme Loser
January 1999
avatar

The relationship between the current target bitmap and the current display can be rather confusing; setting one will sometimes imply the other (can't remember without looking up the docs)

I think the proper way to handle things is to make sure there are no implicit changes. The only exception I would make is that a call to al_create_display would set the target bitmap if it is currently null.

Other than that, you have to explicitly change the display and bitmap yourself.

Quote:

Could we do with only having the current target bitmap? If the target bitmap is the front/back-buffer of a display, then that display is the "current" display.

So how do you switch between two displays if al_set_current_display is removed?

ALLEGRO_DISPLAY *d1 = al_create_display(w,h);
ALLEGRO_BITMAP *bmp1 = al_get_backbuffer();
ALLEGRO_DISPLAY *d2 = al_create_display(w,h);
ALLEGRO_BITMAP *bmp2 = al_get_backbuffer();

// al_set_current_display(d1);
al_set_active_bitmap(bmp1);

It seems more confusing to have to keep a copy of the backbuffer around just to switch to the display.

Or change it so display api functions take ALLEGRO_DISPLAYs

They would have to if you wanted to avoid the above code. But then you have a display API that is only partially state based.

SiegeLord said:

I for one would prefer not to have the al_get_backbuffer functions at all, since the bitmaps returned by those functions are very limited (see the docs [alleg.sourceforge.net]) and have al_set_target_bitmap(0) make the target bitmap be the backbuffer again (some special casing/functioning can be done for the frontbuffer).

I have thought the same sort of thing. I would use specific functions like al_select_backbuffer() and al_select_frontbuffer() to avoid accidental problems with null bitmaps. (Do you really even need to use the front buffer?)

SiegeLord
Member #7,827
October 2006
avatar

I think the overwhelmingly common use case of the front buffer is for front buffer-only applications... I can't imagine right now a situation where it would make sense to use the front buffer when you have the back buffer already there.

"For in much wisdom is much grief: and he that increases knowledge increases sorrow."-Ecclesiastes 1:18
[SiegeLord's Abode][Codes]:[DAllegro5]:[RustAllegro]

Matthew Leverton
Supreme Loser
January 1999
avatar

A function like al_select_display_buffer() could work for single and double buffers. (By rule then, the front buffer would be inaccessible on double buffers.)

Back to the other topic... Maybe the current display shouldn't state based (e.g., al_flip_display(display)). The recent changes to the color have Allegro moving back the other way. :-X

Trent Gamblin
Member #261
April 2000
avatar

I was against the blend color as a parameter :(.

Thomas Fjellstrom
Member #476
June 2000
avatar

The recent changes to the color have Allegro moving back the other way. :-X

Gotta find a balance. Being too state based is just as inconvenient as passing a bajillion parameters around.

--
Thomas Fjellstrom - [website] - [email] - [Allegro Wiki] - [Allegro TODO]
"If you can't think of a better solution, don't try to make a better solution." -- weapon_S
"The less evidence we have for what we believe is certain, the more violently we defend beliefs against those who don't agree" -- https://twitter.com/neiltyson/status/592870205409353730

SiegeLord
Member #7,827
October 2006
avatar

I think the current breakdown of what is a state and what is not to be pretty good (I sort of like the color change, myself, although those tinted functions do look obnoxious).

"For in much wisdom is much grief: and he that increases knowledge increases sorrow."-Ecclesiastes 1:18
[SiegeLord's Abode][Codes]:[DAllegro5]:[RustAllegro]

Thomas Fjellstrom
Member #476
June 2000
avatar

SiegeLord said:

I think the current breakdown of what is a state and what is not to be pretty good (I sort of like the color change, myself, although those tinted functions do look obnoxious).

Yeah, though I can see a good case for removing the display TLS state, and just passing ALLEGRO_DISPLAYs around when needed.

--
Thomas Fjellstrom - [website] - [email] - [Allegro Wiki] - [Allegro TODO]
"If you can't think of a better solution, don't try to make a better solution." -- weapon_S
"The less evidence we have for what we believe is certain, the more violently we defend beliefs against those who don't agree" -- https://twitter.com/neiltyson/status/592870205409353730

Peter Wang
Member #23
April 2000

Elias said:

I assume technically it just means replacing ALLEGRO_DISPLAY with ALLEGRO_BITMAP throughout the API.

No, I wouldn't want that, and I don't think it's necessary.

removing the display TLS state, and just passing ALLEGRO_DISPLAYs around when needed

Yes, exactly. I'm wondering if we need the "current display" concept at all.

Thomas Fjellstrom
Member #476
June 2000
avatar

Yes, exactly. I'm wondering if we need the "current display" concept at all.

I don't think we need it. Theres only a few times you want to even use a function on them, so its not a big deal. And it would simplify the api a bit.

--
Thomas Fjellstrom - [website] - [email] - [Allegro Wiki] - [Allegro TODO]
"If you can't think of a better solution, don't try to make a better solution." -- weapon_S
"The less evidence we have for what we believe is certain, the more violently we defend beliefs against those who don't agree" -- https://twitter.com/neiltyson/status/592870205409353730

Peter Wang
Member #23
April 2000

Ok, I think the problem is that the OpenGL rendering context is tied up with ALLEGRO_DISPLAY (which also deals with other things). What al_set_current_display really does is change the OpenGL rendering context. But al_set_target_bitmap also does it as necessary, by changing the current display...

Proposal:

  • Remove al_set_current_display().


  • Make functions take ALLEGRO_DISPLAY arguments where required, e.g.

    ALLEGRO_BITMAP *al_get_backbuffer(ALLEGRO_DISPLAY *display);
    void al_set_display_icon(ALLEGRO_DISPLAY *display, ALLEGRO_BITMAP *bmp);

  • Remove al_get_current_display(), but replace it with al_get_target_display().
    This basically returns al_get_target_bitmap()->display;

  • al_set_target_bitmap(video_bitmap) internally sets the OpenGL rendering context to the context of the display which owns the bitmap. If the rendering context is already the correct one, nothing happens. The user normally doesn't need to know about rendering contexts, but it useful to know for performance and unavoidable for multithreaded programs. So, no change from current behaviour.


  • al_set_target_bitmap(NULL) immediately releases the OpenGL rendering context.
    This replaces al_set_current_display(NULL).

  • al_set_target_bitmap(memory_bitmap) does not release the OpenGL rendering context, for performance reasons (although it probably doesn't matter). No change from current behaviour.

  • al_destroy_display(display) implicitly calls al_set_target_bitmap(NULL) when al_get_target_bitmap()==display.


  • [Not 100% sure] Rename al_flip_display() to al_flip_target_bitmap(). It swaps the front/back-buffers of the display that owns the target bitmap, if any. Otherwise does nothing.

Amendment:

  • I've started implementing this, and found that al_set_target_bitmap(al_get_backbuffer(display)) occurs so often that a shorthand would be welcome, e.g. al_select_backbuffer(display) as mentioned above.

Mark Oates
Member #1,146
March 2001
avatar

I am completely in favor of the changes proposed in the first four bullet points. I don't know enough about rendering contexts or allegro's inner workings to comment on the latter four.

--
Visit CLUBCATT.com for cat shirts, cat mugs, puzzles, art and more <-- coupon code ALLEGRO4LIFE at checkout and get $3 off any order of 3 or more items!

AllegroFlareAllegroFlare DocsAllegroFlare GitHub

Elias
Member #358
May 2000

I think 99% of all A5 programs will use a single window. Especially in a game it probably won't feel good having more and also in applications it may be bad design. (Isn't it the main complaint about Gimp that it uses multiple windows?)

With that in mind in almost all cases there wouldn't be any confusion about the current display as it naturally would be the one existing display, over the lifetime of the application.

Only in the rare case where you have more than one display would you ever want to call al_set_current_display. No functions actually would need to take an explicit ALLEGRO_DISPLAY parameter. I think the confusion right now is only because some functions like al_get_window_position or al_acknowledge_resize or al_get_display_event_source use such a parameter when they aren't supposed to.

So I think fixing those three functions by adding the explicit parameter to all other functions isn't the ideal solution - I'd much rather simply remove it from them instead and use the current display.

However I don't feel strongly about it. Completely hiding ALLEGRO_DISPLAY indeed would go too far as in multi-window applications only being able to change it with al_set_target_bitmap would be confusing. So we need the ALLEGRO_DISPLAY type in which case we may as well pass it around.

--
"Either help out or stop whining" - Evert

Peter Wang
Member #23
April 2000

It turns out you run into silly problems even with a single display, if you use multiple threads.

Here's an example. In ex_native_filechooser, we create a separate thread to call al_show_native_file_dialog(), which (undocumentedly) uses the current display as the parent window. But the display is also current for the parent thread, which is invalid according to our API. So to do it correctly you need to call al_set_current_display(NULL) in the parent thread, create the child thread, set the current display in the child thread, call al_show_native_file_dialog(), then release the display again. The child thread didn't even want the display's rendering context.

Elias
Member #358
May 2000

I see. Probably also a rare case though. I wonder if the upcoming change can fix the technical troubles of the native dialogs addon... I don't think using the current display actually ever worked - here X11 seems to create detached windows (i.e. they go behind the A5 windows which they should not) anyway. Most likely would need to create the window from the same thread where the X11 window was created since X11 itself doesn't handle multiple threads so well.

--
"Either help out or stop whining" - Evert

Matthew Leverton
Supreme Loser
January 1999
avatar

So does any of this make al_get_window_position work?

With the latest SVN, how are you supposed to do this:

al_set_target_bitmap(al_get_backbuffer());

when you don't know what the current display is? The commit log says:

Quote:

al_{g,s}et_current_display are still present for now, but only used
internally.


where for now implies it won't be available in the future.

Thomas Fjellstrom
Member #476
June 2000
avatar

the al_get_backbuffer function or one like it would have to take a ALLEGRO_DISPLAY arg.

--
Thomas Fjellstrom - [website] - [email] - [Allegro Wiki] - [Allegro TODO]
"If you can't think of a better solution, don't try to make a better solution." -- weapon_S
"The less evidence we have for what we believe is certain, the more violently we defend beliefs against those who don't agree" -- https://twitter.com/neiltyson/status/592870205409353730

 1   2 


Go to: