Patch to add window constraint functions
jmasterx

I have made a patch for the latest 5.1 svn that adds:

void al_set_window_constraints(ALLEGRO_DISPLAY *display, int min_w, int min_h, int max_w, int max_h)

and

void al_get_window_constraints(ALLEGRO_DISPLAY *display, int *min_w, int *min_h, int *max_w, int *max_h )

to Windows, OSX, and X.

It seems to be working fine on Windows. I have not tested it on OSX or Linux / X.

Hopefully this can make it to the SVN :)

I was unable to attach it through a.cc so I have uploaded it here:
http://www.mediafire.com/?46w947jk3rcjic7

Edit:
Fixed version that fixes OSX support:
http://www.mediafire.com/?7m0l5v3pd550kqt

Edit2:
Revision 3 fixes OSX support again.
http://www.mediafire.com/?h4n2m5homhhbv9l

Edit3
Revision 4 fixes Linux support
http://www.mediafire.com/?299l97yrrooazgo

Edit4
Revision 5 makes them boolean functions, formatted the code
http://www.mediafire.com/?wx8kz3zor3f9ekv

torhu

pastebin.com would be a good place to upload something like this.

Trent Gamblin

Do you have an example I can use to test it on OSX?

jmasterx

I tested it on Windows by modifying ex_resize:

#SelectExpand
1#include "allegro5/allegro.h" 2#include <allegro5/allegro_primitives.h> 3 4#include "common.c" 5 6static void redraw(void) 7{ 8 ALLEGRO_COLOR black, white; 9 int w, h; 10 11 white = al_map_rgba_f(1, 1, 1, 1); 12 black = al_map_rgba_f(0, 0, 0, 1); 13 14 al_clear_to_color(white); 15 w = 100; 16 h = 100; 17 al_draw_line(0, h, w / 2, 0, black, 0); 18 al_draw_line(w / 2, 0, w, h, black, 0); 19 al_draw_line(w / 4, h / 2, 3 * w / 4, h / 2, black, 0); 20 al_flip_display(); 21} 22 23int main(void) 24{ 25 ALLEGRO_DISPLAY *display; 26 ALLEGRO_TIMER *timer; 27 ALLEGRO_EVENT_QUEUE *events; 28 ALLEGRO_EVENT event; 29 int rs = 100; 30 bool resize = false; 31 32 /* Initialize Allegro and create an event queue. */ 33 if (!al_init()) { 34 abort_example("Could not init Allegro.\n"); 35 return 1; 36 } 37 al_init_primitives_addon(); 38 events = al_create_event_queue(); 39 40 /* Setup a display driver and register events from it. */ 41 al_set_new_display_flags(ALLEGRO_RESIZABLE); 42 display = al_create_display(640, 480); 43 if (!display) { 44 abort_example("Could not create display.\n"); 45 return 1; 46 } 47 al_set_window_constraints(display,640,0,0,600); 48 al_register_event_source(events, al_get_display_event_source(display)); 49 50 timer = al_create_timer(0.1); 51 al_start_timer(timer); 52 53 /* Setup a keyboard driver and register events from it. */ 54 al_install_keyboard(); 55 al_register_event_source(events, al_get_keyboard_event_source()); 56 al_register_event_source(events, al_get_timer_event_source(timer)); 57 58 /* Display a pulsating window until a key or the closebutton is pressed. */ 59 redraw(); 60 while (true) { 61 if (true) { 62 int s; 63 rs += 10; 64 if (rs == 300) 65 rs = 100; 66 s = rs; 67 if (s > 200) 68 s = 400 - s; 69 //al_resize_display(display, s, s); 70 redraw(); 71 resize = false; 72 } 73 al_wait_for_event(events, &event); 74 if (event.type == ALLEGRO_EVENT_TIMER) { 75 resize = true; 76 } 77 else if (event.type == ALLEGRO_EVENT_DISPLAY_CLOSE) { 78 break; 79 } 80 else if (event.type == ALLEGRO_EVENT_KEY_DOWN) { 81 break; 82 } 83 else if (event.type == ALLEGRO_EVENT_DISPLAY_RESIZE) { 84 al_acknowledge_resize(display); 85 86 } 87 } 88 89 return 0; 90} 91 92/* vim: set sts=4 sw=4 et: */

That way it is minimum restricted on the x axis and maximum restricted on the y, zero means no restriction.

Now that I think about it, it may be nessesary to call al_resize_display(display, display->w, display->h) after to sync up the backbuffer.

If you need to make changes to the patch to get OSX to work just let me know ad Ill edit the first post.

Trent Gamblin

The patch has no effect on OSX. One thing that should be done here if you're going to fix this is to make these display options set with al_set_new_display_option. I don't think we need another function, that's what display options are for... someone correct me if I'm wrong.

jmasterx

I just went with what Matthew had suggested. It is possible that for certain scenarios that the user may want to change this after the display is created.

I'll boot into OSX and try some things to see if I can get it working.

Trent Gamblin
jmasterx said:

I just went with what Matthew had suggested. It is possible that for certain scenarios that the user may want to change this after the display is created.

Yeah, that is true.. so it's probably best as you have it.

jmasterx

FIXED IT :D. I forgot to actually assign the min and max values to the display -_-'.
I'll boot back to Windows and give you the new patch.

Edit:
Here's the new revised version, confirmed working on OSX.
http://www.mediafire.com/?7m0l5v3pd550kqt

Unfortunately, I do not have Linux native installed. I only have it in a VM and I cannot seem to get an Allegro window created in the VM so I cannot test it for Linux. If someone could try it and report back It'd be appreciated.

Trent Gamblin

I was checking to make sure the actual size of the window was exactly 640x600 and I got this image (attached). If you look at it closely you can see there are 3 pixels in each bottom corner that are transparent. Any idea why that would be?

jmasterx

I really do not know why that would happen. What did you use to capture the image? I'll try to reproduce it here.

In the mean time, it seems that the OSX window does not sem to apply the constraints when al_resize_display is called, so I will try to address this.

Edit:
Here is revision 3.
OSX seems to be working as good as Windows now.
http://www.mediafire.com/?h4n2m5homhhbv9l

I do not know if this fixes your 3 pixel issue but it might since I fixed al_resize_display to work with constraints on osx.

I guess I'll install Linux native on my PC and get the Linux version working properly so it can be committed today and I can forget about it and check it off my todo list.

Matthew Leverton

I don't think we need another function, that's what display options are for... someone correct me if I'm wrong.

There's always a bit of a struggle between those who favor of monolithic options and those who favor explicit functions. I generally prefer explicit functions, so of course I suggested an explicit function.

I think display options should be limited to those things that affect the creation of the display (or are very unlikely to ever be needed.) That is, here are a list of properties that I require or want, and let me know if you can create the display. It shouldn't be about minimizing the number of functions for the sake of minimizing functions.

Constraints, IMO, do not fall under either of those restrictions and therefore do deserve a dedicated function.

jmasterx said:

In the mean time, it seems that the OSX window does not sem to apply the constraints when al_resize_display is called, so I will try to address this.

So is al_resize_display() meant to be constrained? I don't know that it really needs to be, if it's not something that's easily supported. The main benefit of constraints is for abstract resizing from the user.

jmasterx

It was easier to support constraining for al_resize_display.

I just got done proper Linux support. With this new revision, all 3 platforms seem to show consistent results which means it should be pretty much ready for the svn, I've tested on all 3 platforms.

Here it is:
http://www.mediafire.com/?299l97yrrooazgo

Peter Wang

al_set_window_constraints and al_get_window_constraints should return a boolean indicating success.

Add documentation and fix the coding style please. It's not that hard.

I still would like a constrain-aspect-ratio display flag. This might be a good time to add it.

jmasterx

How is failure determined? I did not think it was possible for it to fail?

Matthew Leverton

It might return false on full screen or on certain mobile devices, or if you send invalid parameters like -42 or a positive value that's too small.

jmasterx

Then why doesn't al_set_window_position return success? You cannot move a window in fullscreen nor can you move it to a ridiculous place like -2000,-2000.

Peter Wang

It should, too. Not everything was scrutinised as it might have been when 5.0 was developed.

jmasterx

So then I should return false on unsupported platforms, if any of the values are < 0 and return false if the fullscreen flag is set? Are there any other ways it can fail?

For the documentation I'll add more comments.

I'm not sure how to address coding style. What needs to change? Would you rather:

if(something)
{
   x = something;
}
else
{
  x = something_else;
}

over

if(something)
x = something;
else x = something_else;

Do you think this functionality will make it into 5.05? I and I'm sure others would find it useful to add to their current projects.

Peter Wang

Max might be smaller than min? Maybe some platforms have other restrictions, e.g. must be multiple of four.

Documentation means adding text to docs/src/refman/display.txt

For code style, just look at the code around you: K&R with 3 space indents (no hard tabs in new code), and no cuddled else. Space after if, while, for, etc. keywords. I prefer lines wrapped at 80 chars; this is mandatory for code which is copied into documentation (e.g. prototypes after Function: headers). Capitalise and punctuate comments.

For your example:

if (something) {
   x = something;
}
else {
   x = something_else;
}

SiegeLord
jmasterx said:

For the documentation I'll add more comments.

Comments are not documentation. You need to document the functions in docs/src/refman/display.txt

Quote:

I'm not sure how to address coding style.

I've noted three things. You used tabs instead of spaces (Allegro source uses 3 spaces per indentation level). You've used C++ comments in C files (// instead of /* */) Also, use attached braces, like so:

if (blah) {

}

instead of

if (blah)
{

}

jmasterx

Alright, I'll see if I can address those issues. I've gotten too used to auto formatting by the IDE.

I've never coded in pure C, so this is a new experience for me.

Edit:
I have made them boolean functions.
I have cleaned up and standardized the code as well as I could.
I have documented the functions.

It should be good for the svn now.

Here is the link:
http://www.mediafire.com/?wx8kz3zor3f9ekv

Edit2:
Is there any particular reason this has not made it into the svn yet? If there's anything still wrong with it, or any additional tests I need to run I'll gladly do it. Someone just needs to let me know.

Thanks

SiegeLord

Well, the patch still has hard tabs in quite a few places. Surely your IDE has an option to show whitespace? The actual reason is probably the people are a bit busy :).

Trent Gamblin

We need a new example for this. You can start with ex_resize2. Hook it up in cmake. Still need to know why those pixels are missing too. But yeah, it's just people being busy.

jmasterx

Alright, tonight or tomorrow I'll try to address those issues.
I'll make an example ex_window_constraints
and I will also try my best to remove those darn tabs, but it is tricky since Visual Studio and Visual Assist like to add tabs.

torhu

MSVC has an untabify command on the Edit->Advanced menu. And you can press Ctrl-Shift-8 to show whitespace.

jmasterx

Here is (what I hope is) the final version of the patch.

This one includes an example, and no more tabs. The example has been tested on all 3 platforms and performs consistently across all 3.

The example lets you press keys to toggle constraints so that is tested too.
It also displays the resolution and the result of al_get_constraints for easy debugging.

Here it is:
http://www.mediafire.com/?l7h595pp6cbtz67
{"name":"605085","src":"\/\/djungxnpq2nug.cloudfront.net\/image\/cache\/f\/9\/f99ee73ef41ecf75d04af0b10aa5d1eb.png","w":800,"h":600,"tn":"\/\/djungxnpq2nug.cloudfront.net\/image\/cache\/f\/9\/f99ee73ef41ecf75d04af0b10aa5d1eb"}605085

Peter Wang

Where did the 9000 come from?

jmasterx

I needed some kind of value, I do not know how to get the maximum window size but I figure a window won't be bigger than 9000. Is there a better way?

Peter Wang

I don't know off the top of my head.

Also, the constraints don't apply to maximisation, at least on X. Should they? The docs don't say. I don't know if it's even possible, short of disabling the maximise button.

Anyway, the patch is in my local workspace with additional formatting fixes.

Matthew Leverton

Also, the constraints don't apply to maximisation, at least on X. Should they?

On Windows and Linux, I would say no. They have a proper maximized state. (Windows does, at least. I guess it may vary on the window manager for Linux.) If the maximize button isn't wanted, then it should be disabled. I don't think Allegro should override it to provide non-standard functionality.

But OS X has that silly zoom button, so I think the constraint should apply there.

REVISION:

It appears that the maximize button was thrown into ALLEGRO_RESIZABLE, even though they are two distinct concepts. There should be two flags that control them separately.

jmasterx

On Windows and OSX, pressing the zoom, maximize etc maintains the constraints, and on Mint Linux for me maximizing respects the constraints, but remember, these are hints so the underlying window manager might not listen.

Here is what it looks like for me. Left is "maximized" right is not.

{"name":"605089","src":"\/\/djungxnpq2nug.cloudfront.net\/image\/cache\/3\/9\/393320cafed0b24dd8925835aea2b4ff.png","w":1680,"h":1050,"tn":"\/\/djungxnpq2nug.cloudfront.net\/image\/cache\/3\/9\/393320cafed0b24dd8925835aea2b4ff"}605089

Peter Wang

I've committed it now with some minor changes.

I saw on the Apple documentation that the window environment restricts windows to 10000 pixels so I replaced the 9000 with that. It also says the default is FLT_MAX so probably that's what we should be using in the no-constraint case. Can you check that and send a patch?

For X, I made it not set any size hints if all constraints are disabled. Again, removed the arbitrary limit of 9000.

jmasterx

Here is the patch.

I removed your comments about FLT_MAX and replaced 10000 with FLT_MAX. It compiled and works as well as before.

http://www.allegro.cc/files/attachment/605098

I was also wondering if this function will be able to go in the 5.0 branch (5.0.6) or if it breaks binary compatibility and thus must go into 5.1?

Peter Wang

It could. I've been considering a 5.0.6 release to backport some minor feature additions. It would be the first time we've done that, so I'd want to be doubly sure we don't break backwards compatibility.

jmasterx

I think as long as you do not change 5.0 function signatures I do not see how it could break backwards compatibility. I think dynamic libraries are intended with the idea of adding more functionality / functions without breaking the old.

Since things like ALLEGRO_DSPLAY are opaque data types to the user, the additions of data into ALLEGRO_DISPLAY should not be a problem.

Peter Wang

Of course, but shit happens.

jmasterx

You could get it tested by the community before officially releasing it. I'd be glad to test it out with some of my projects and see if it is still binary compatible.

Peter Wang

ex_window_constraints shows only the red background if I use video bitmaps and the Direct3D driver. Can you confirm this and investigate?

jmasterx

It seems this might be an Allegro bug. If I call al_resize_display before al_flip_display and after al_set_new_bitmap_flags(ALLEGRO_VIDEO_BITMAP) a call to al_resize_display seems to cause the backbuffer to stop working.

To 'fix' the example I simply moved the call to al_set_window_constraints to before setting the bitmap flag.

This bug shouldn't really affect developers since calls to al_resize_display and al_set_window_constraints are usually done before setting bitmap flags and after flipping the display but someone might eventually want to look into this.

Thread #608744. Printed from Allegro.cc