June 2009 - Wine-Devel - list.winehq.org

Re: gdiplus: add support for string alignment to GdipDrawString (try 2)

by Stephan Rose

>(reply didn't go to list as I expected, grr..) > >> Naw, it should still work normally. I've done aligned & rotated text under >windows before with DrawTextW, you still pass the rect like you would if it >wasn't rotated and set the alignment flags. The world transform should take >care of getting it all rotated to where it is supposed to be with the >alignments in the right place. > >Maybe, but builtin gdiplus has its own world transform and doesn't use >the gdi32 one. That's what transform_and_round_points is for. True but you're still using the GDI32 DrawText to actually render the text. transform_and_round_points is only doing what it says, transforming the coordinates. If you rotate the text by 90 degrees, DrawText is still going to have to be aware that the text is to be rotated 90 degrees to draw it correctly or else you'll get your text at the right position, but the text direction will be wrong. Unless I'm overlooking something? > >> Ok question then, how do I separate this into two patches? Never done >anything with patches like this before so this is a first for me. > >Just make two commits in Git, and format-patch will create two patch files. > That makes sense but, one question there. Does this mean I'm going to have to restore the file to it's original state, then add the one change, commit, add the other change, then commit again? With a change of this size, that isn't that big a deal. But on larger changes I could see that being a bit problematic. But maybe I'm just not used to it. =) Thanks, Stephan

June 2, 2009

Re: gdiplus: add support for string alignment to GdipDrawString (try 2)

by Vincent Povirk

(reply didn't go to list as I expected, grr..) > Naw, it should still work normally. I've done aligned & rotated text under windows before with DrawTextW, you still pass the rect like you would if it wasn't rotated and set the alignment flags. The world transform should take care of getting it all rotated to where it is supposed to be with the alignments in the right place. Maybe, but builtin gdiplus has its own world transform and doesn't use the gdi32 one. That's what transform_and_round_points is for. > Ok question then, how do I separate this into two patches? Never done anything with patches like this before so this is a first for me. Just make two commits in Git, and format-patch will create two patch files.

June 2, 2009

Re: gdiplus: add support for string alignment to GdipDrawString (try 2)

by Stephan Rose

>Without doing any actual testing: > >The vertical alignment stuff looks right to me. > >Horizontal alignment is suspicious. If the world transform rotates the >text 90 degrees, DrawTextW can't be getting enough information to >center the text. Then again, I'm not sure if rotation works properly >with GdipDrawString in general. I'd write a test program for rotated >text. Naw, it should still work normally. I've done aligned & rotated text under windows before with DrawTextW, you still pass the rect like you would if it wasn't rotated and set the alignment flags. The world transform should take care of getting it all rotated to where it is supposed to be with the alignments in the right place. But I agree with you though, a test with rotated text is needed. If necessary though it isn't a big deal for me to change things to align without using DT_CENTER and DT_RIGHT. I'd want to test with the alignment flags first though as it keeps the code simpler if I can stick with those. > >I would probably send this as a series of 2 patches with vertical >alignment first. Ok question then, how do I separate this into two patches? Never done anything with patches like this before so this is a first for me. > >In spite Paul Vriens' comments, don't change the formatting of code >you're not changing. I guess the existing if's are already >inconsistent enough that you can get away with either style (and that >might actually be my fault). Without looking at it, I think it was only two if statements that weren't mine that I modified formatting on. Figured my changes to the function were major enough to where getting those two consistent with everything else wouldn't matter much. Up to you all if you want me to just leave those untouched or get their formatting in line with everything else. Either way's fine by me. =) Stephan

June 2, 2009

Re: gdiplus: add support for string alignment to GdipDrawString (try 2)

by Vincent Povirk

Without doing any actual testing: The vertical alignment stuff looks right to me. Horizontal alignment is suspicious. If the world transform rotates the text 90 degrees, DrawTextW can't be getting enough information to center the text. Then again, I'm not sure if rotation works properly with GdipDrawString in general. I'd write a test program for rotated text. I would probably send this as a series of 2 patches with vertical alignment first. In spite Paul Vriens' comments, don't change the formatting of code you're not changing. I guess the existing if's are already inconsistent enough that you can get away with either style (and that might actually be my fault). Vincent Povirk On Tue, Jun 2, 2009 at 7:12 AM, Stephan Rose<kermos(a)somrek.net> wrote: > - Adds support for aligning a string inside the given bounding rectangle > for GdipDrawString. Both vertical and horizontal alignments implemented. > > - Fixed code formatting issues (tabs / spaces, location of braces) to be > consistent with existing code. > > > > > >

June 2, 2009

Re: Website: Point the Development pages entirely to the Wiki.

by Dimi Paun

On Tue, 2009-06-02 at 09:10 -0500, Jeremy Newman wrote: > Scott Ritchie wrote: > > Jeremy Newman wrote: > >> I'm holding this patch hostage until the Wiki is updated with the > new > >> theme. > > > > Aren't you the one who's updating the wiki to the theme? > > No. I currently do not have access to the Wiki. That is hosted by > elsewhere. This is true, I'm hosting it. And while I said I'll look at upgrading to the new theme, I just didn't have the time. However, the code is freely available to everybody on SourceForge: http://wine.cvs.sourceforge.net/viewvc/wine/wiki/ And I'm more than happy to accept patches. -- Dimi Paun <dimi(a)lattica.com> Lattica, Inc.

June 2, 2009

Re: Website: Point the Development pages entirely to the Wiki.

by Jeremy Newman

Scott Ritchie wrote: > Jeremy Newman wrote: >> I'm holding this patch hostage until the Wiki is updated with the new >> theme. > > Aren't you the one who's updating the wiki to the theme? No. I currently do not have access to the Wiki. That is hosted by elsewhere. We are willing, and have the space/cpu to host the wiki. -Newman

June 2, 2009

Re: Implications for Wine from the Ubuntu Developer Summit -- Pulseaudio

by Susan Cragin

>2009/6/2 Scott Ritchie <scott(a)open-vote.org>: >> First, I talked with a Pulseaudio expert about what we can do to make >> things work better. He said that if we want good compatibility we will >> need our ALSA stack to use the Pulseaudio safe subset: >> http://0pointer.de/blog/projects/guide-to-sound-apis.html. I've filed a >> metabug tracking this here: >> http://bugs.winehq.org/show_bug.cgi?id=18740. Use of this unsafe subset >> can cause most problems with stuttering or even complete dropoff. > >As far as I know, this is not possible for Wine (without massive >latency issues caused by overbuffering in Wine itself) due to the fact >that Wine has to make DirectSound apps happy. It's also not very >consoling from this Pulseaudio expert that they don't seem to see a >problem with the ALSA layer on their end - "it's the apps that need to >be fixed". Wasn't the point of Pulse that you could use libasound/OSS >apps without modification? :) > >> I'm not completely familiar with how sound works in Wine, but in the >> past I remember that one complaint about PulseAudio over ALSA was >> latency. Latency issues these days are mostly due to bad kernel >> configurations > >PulseAudio will *always* have more latency than ALSA. This has nothing >to do with kernel configurations, just that using a >CONFIG_NO_HZ/CONFIG_HZ_1000 (or whatever it is) kernel from the -rt >tree makes the latency issue less severe. New kernel won't fix Wine + >Pulse problem. Forgive me in advance for interjecting dross into a technical discussion. Would part of the problem be solved if users could be given the option of installing pulseaudio? That way, some users would be made instantly happy. There is a subset of users (albeit a tiny subset) that just needs one sound application running, but wants that application running with crystal-clear sound and near-zero latency. These are certain professional audio users, and those who run Dragon NaturallySpeaking through wine. Those people may be ill-served by having pulseaudio at all. Right now, in wine, removing pulseaudio completely doesn't seem to return alsa's functionality. Wine/alsa freezes, and I have to use OSS emulation, but OSS just does not produce as high-quality a sound. PS -- The rt kernel really is wonderful for speech recognition or any other use. The repository version has problems that are being worked on. In the meantime, just compile your own.

June 2, 2009

Re: [2/5] libwine: Add a generic red-black tree.

by Henri Verbeet

Alexandre Julliard wrote: > You can't hardcode malloc(), the allocator needs to be configurable, > particularly for use from the memory management code. Also printing an > ERR in that sort of code is not acceptable, errors need to be propagated > correctly and handled by the user. > > I also still think this would be better as inline functions, at least > until it stabilizes. We don't want to change the libwine interface any > more than strictly necessary. > How about something like the attached patch? commit 2d1c3777494fe7bf31283c4c51ced3ea83093d2c Author: Henri Verbeet <hverbeet(a)codeweavers.com> Date: Tue Jun 2 15:14:18 2009 +0200 include: Add a generic red-black tree. diff --git a/include/wine/rbtree.h b/include/wine/rbtree.h new file mode 100644 index 0000000..da80b7f --- /dev/null +++ b/include/wine/rbtree.h @@ -0,0 +1,368 @@ +/* + * Red-black search tree support + * + * Copyright 2009 Henri Verbeet + * Copyright 2009 Andrew Riedi + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#ifndef __WINE_WINE_RBTREE_H +#define __WINE_WINE_RBTREE_H + +#define WINE_RB_ENTRY_VALUE(element, type, field) \ + ((type *)((char *)(element) - FIELD_OFFSET(type, field))) + +struct wine_rb_entry +{ + struct wine_rb_entry *left; + struct wine_rb_entry *right; + unsigned int flags; +}; + +struct wine_rb_allocator +{ + void *(*alloc)(size_t size); + void *(*realloc)(void *ptr, size_t size); + void (*free)(void *ptr); +}; + +struct wine_rb_stack +{ + struct wine_rb_allocator allocator; + struct wine_rb_entry ***entries; + size_t count; + size_t size; +}; + +typedef int (wine_rb_compare_func_t)(const void *key, const struct wine_rb_entry *entry); +typedef void (wine_rb_traverse_func_t)(struct wine_rb_entry *entry, void *context); + +struct wine_rb_tree +{ + wine_rb_compare_func_t *compare; + struct wine_rb_entry *root; + struct wine_rb_stack stack; +}; + +#define WINE_RB_FLAG_RED 0x1 +#define WINE_RB_FLAG_STOP 0x2 +#define WINE_RB_FLAG_TRAVERSED_LEFT 0x4 +#define WINE_RB_FLAG_TRAVERSED_RIGHT 0x8 + +static inline int wine_rb_is_red(struct wine_rb_entry *entry) +{ + return entry && (entry->flags & WINE_RB_FLAG_RED); +} + +static inline void wine_rb_stack_clear(struct wine_rb_stack *stack) +{ + stack->count = 0; +} + +static inline void wine_rb_rotate_left(struct wine_rb_entry **entry) +{ + struct wine_rb_entry *e = *entry; + struct wine_rb_entry *right = e->right; + + e->right = right->left; + right->left = e; + right->flags &= ~WINE_RB_FLAG_RED; + right->flags |= e->flags & WINE_RB_FLAG_RED; + e->flags |= WINE_RB_FLAG_RED; + *entry = right; +} + +static inline void wine_rb_rotate_right(struct wine_rb_entry **entry) +{ + struct wine_rb_entry *e = *entry; + struct wine_rb_entry *left = e->left; + + e->left = left->right; + left->right = e; + left->flags &= ~WINE_RB_FLAG_RED; + left->flags |= e->flags & WINE_RB_FLAG_RED; + e->flags |= WINE_RB_FLAG_RED; + *entry = left; +} + +static inline void wine_rb_flip_color(struct wine_rb_entry *entry) +{ + entry->flags ^= WINE_RB_FLAG_RED; + entry->left->flags ^= WINE_RB_FLAG_RED; + entry->right->flags ^= WINE_RB_FLAG_RED; +} + +static inline void wine_rb_fixup(struct wine_rb_stack *stack) +{ + while (stack->count) + { + struct wine_rb_entry **entry = stack->entries[stack->count - 1]; + + if ((*entry)->flags & WINE_RB_FLAG_STOP) + { + (*entry)->flags &= ~WINE_RB_FLAG_STOP; + return; + } + + if (wine_rb_is_red((*entry)->right) && !wine_rb_is_red((*entry)->left)) wine_rb_rotate_left(entry); + if (wine_rb_is_red((*entry)->left) && wine_rb_is_red((*entry)->left->left)) wine_rb_rotate_right(entry); + if (wine_rb_is_red((*entry)->left) && wine_rb_is_red((*entry)->right)) wine_rb_flip_color(*entry); + --stack->count; + } +} + +static inline void wine_rb_move_red_left(struct wine_rb_entry **entry) +{ + wine_rb_flip_color(*entry); + if (wine_rb_is_red((*entry)->right->left)) + { + wine_rb_rotate_right(&(*entry)->right); + wine_rb_rotate_left(entry); + wine_rb_flip_color(*entry); + } +} + +static inline void wine_rb_move_red_right(struct wine_rb_entry **entry) +{ + wine_rb_flip_color(*entry); + if (wine_rb_is_red((*entry)->left->left)) + { + wine_rb_rotate_right(entry); + wine_rb_flip_color(*entry); + } +} + +static inline int wine_rb_stack_push(struct wine_rb_stack *stack, struct wine_rb_entry **entry) +{ + if (stack->count == stack->size) + { + size_t new_size = stack->size << 1; + struct wine_rb_entry ***new_entries = stack->allocator.realloc(stack->entries, + new_size * sizeof(*stack->entries)); + + if (!new_entries) return -1; + + stack->entries = new_entries; + stack->size = new_size; + } + + stack->entries[stack->count++] = entry; + return 0; +} + +static inline void wine_rb_clear_traverse_stack(struct wine_rb_stack *stack) +{ + unsigned int i; + + for (i = 0; i < stack->count; ++i) + { + (*stack->entries[i])->flags &= ~(WINE_RB_FLAG_TRAVERSED_LEFT | WINE_RB_FLAG_TRAVERSED_RIGHT); + } + + wine_rb_stack_clear(stack); +} + +static inline int wine_rb_postorder(struct wine_rb_tree *tree, wine_rb_traverse_func_t *callback, void *context) +{ + struct wine_rb_entry **entry; + + if (!tree->root) return 0; + + for (entry = &tree->root;;) + { + struct wine_rb_entry *e = *entry; + + if (e->left && !(e->flags & WINE_RB_FLAG_TRAVERSED_LEFT)) + { + if (wine_rb_stack_push(&tree->stack, entry) == -1) + { + wine_rb_clear_traverse_stack(&tree->stack); + return -1; + } + e->flags |= WINE_RB_FLAG_TRAVERSED_LEFT; + entry = &e->left; + continue; + } + + if (e->right && !(e->flags & WINE_RB_FLAG_TRAVERSED_RIGHT)) + { + if (wine_rb_stack_push(&tree->stack, entry) == -1) + { + wine_rb_clear_traverse_stack(&tree->stack); + return -1; + } + e->flags |= WINE_RB_FLAG_TRAVERSED_RIGHT; + entry = &e->right; + continue; + } + + e->flags &= ~(WINE_RB_FLAG_TRAVERSED_LEFT | WINE_RB_FLAG_TRAVERSED_RIGHT); + callback(e, context); + + if (!tree->stack.count) break; + entry = tree->stack.entries[--tree->stack.count]; + } + + return 0; +} + +static inline int wine_rb_init(struct wine_rb_tree *tree, wine_rb_compare_func_t *compare, + const struct wine_rb_allocator *allocator) +{ + tree->compare = compare; + tree->root = NULL; + + tree->stack.allocator = *allocator; + tree->stack.entries = tree->stack.allocator.alloc(16 * sizeof(*tree->stack.entries)); + if (!tree->stack.entries) return -1; + tree->stack.size = 16; + tree->stack.count = 0; + + return 0; +} + +static inline int wine_rb_for_each_entry(struct wine_rb_tree *tree, wine_rb_traverse_func_t *callback, void *context) +{ + return wine_rb_postorder(tree, callback, context); +} + +static inline int wine_rb_destroy(struct wine_rb_tree *tree, wine_rb_traverse_func_t *callback, void *context) +{ + /* Note that we use postorder here because the callback will likely free the entry. */ + if (callback) + { + if (wine_rb_postorder(tree, callback, context) == -1) return -1; + } + + tree->root = NULL; + tree->stack.allocator.free(tree->stack.entries); + + return 0; +} + +static inline struct wine_rb_entry *wine_rb_get(const struct wine_rb_tree *tree, const void *key) +{ + struct wine_rb_entry *entry = tree->root; + while (entry) + { + int c = tree->compare(key, entry); + if (!c) return entry; + entry = c < 0 ? entry->left : entry->right; + } + return NULL; +} + +static inline int wine_rb_put(struct wine_rb_tree *tree, const void *key, struct wine_rb_entry *entry) +{ + struct wine_rb_entry **parent = &tree->root; + + while (*parent) + { + int c; + + if (wine_rb_stack_push(&tree->stack, parent) == -1) + { + wine_rb_stack_clear(&tree->stack); + return -1; + } + + c = tree->compare(key, *parent); + if (!c) + { + wine_rb_stack_clear(&tree->stack); + return -1; + } + else if (c < 0) parent = &(*parent)->left; + else parent = &(*parent)->right; + } + + entry->flags = WINE_RB_FLAG_RED; + entry->left = NULL; + entry->right = NULL; + *parent = entry; + + wine_rb_fixup(&tree->stack); + tree->root->flags &= ~WINE_RB_FLAG_RED; + + return 0; +} + +static inline int wine_rb_remove(struct wine_rb_tree *tree, const void *key) +{ + struct wine_rb_entry **entry = &tree->root; + int ret = 0; + + while (*entry) + { + if (tree->compare(key, *entry) < 0) + { + if ((ret = wine_rb_stack_push(&tree->stack, entry)) == -1) break; + if (!wine_rb_is_red((*entry)->left) && !wine_rb_is_red((*entry)->left->left)) wine_rb_move_red_left(entry); + entry = &(*entry)->left; + } + else + { + if (wine_rb_is_red((*entry)->left)) wine_rb_rotate_right(entry); + if (!tree->compare(key, *entry) && !(*entry)->right) + { + *entry = NULL; + break; + } + if (!wine_rb_is_red((*entry)->right) && !wine_rb_is_red((*entry)->right->left)) + wine_rb_move_red_right(entry); + if (!tree->compare(key, *entry)) + { + struct wine_rb_entry **e = &(*entry)->right; + struct wine_rb_entry *m = *e; + while (m->left) m = m->left; + + if ((ret = wine_rb_stack_push(&tree->stack, entry)) == -1) break; + (*entry)->flags |= WINE_RB_FLAG_STOP; + + while ((*e)->left) + { + if ((ret = wine_rb_stack_push(&tree->stack, e)) == -1) + { + wine_rb_fixup(&tree->stack); + goto err; + } + if (!wine_rb_is_red((*e)->left) && !wine_rb_is_red((*e)->left->left)) wine_rb_move_red_left(e); + e = &(*e)->left; + } + *e = NULL; + wine_rb_fixup(&tree->stack); + + *m = **entry; + *entry = m; + + break; + } + else + { + if ((ret = wine_rb_stack_push(&tree->stack, entry)) == -1) break; + entry = &(*entry)->right; + } + } + } + +err: + wine_rb_fixup(&tree->stack); + if (tree->root) tree->root->flags &= ~WINE_RB_FLAG_RED; + + return ret; +} + +#endif /* __WINE_WINE_RBTREE_H */

June 2, 2009

Re: Implications for Wine from the Ubuntu Developer Summit

by Scott Ritchie

Roderick Colenbrander wrote: > On Tue, Jun 2, 2009 at 10:08 AM, Francois Gouget <fgouget(a)free.fr> wrote: >> On Tue, 2 Jun 2009, Ben Klein wrote: >> >>> 2009/6/2 Scott Ritchie <scott(a)open-vote.org>: >> [...] >>>> First, I talked with a Pulseaudio expert about what we can do to make >>>> things work better. He said that if we want good compatibility we will >>>> need our ALSA stack to use the Pulseaudio safe subset: >>>> http://0pointer.de/blog/projects/guide-to-sound-apis.html. I've filed a >>>> metabug tracking this here: >>>> http://bugs.winehq.org/show_bug.cgi?id=18740. Use of this unsafe subset >>>> can cause most problems with stuttering or even complete dropoff. >>> As far as I know, this is not possible for Wine (without massive >>> latency issues caused by overbuffering in Wine itself) due to the fact >>> that Wine has to make DirectSound apps happy. >> Which of the DONTs are causing problems for Wine? (And why?) >> It would be nice to identify them to be able to send argumented feedback >> to the PulseAudio developers. >> >> >> -- >> Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ >> tcA thgirypoC muinelliM latigiD eht detaloiv tsuj evah uoY >> >> >> > > I have quickly checked the code and this are I think some of the > 'easy' DONTs we violate: > - we use snd_config_xxx(), this is replaceable by snd_device_name_hint() > - we use snd_card_xxx(), this can be replaced by snd_device_name_hint() > - we hard code device strings (e.g. plughw0) > - we use snd_pcm_avail_update and snd_pcm_delay > (there might be some more but those are less trivial to check) > > The most critical are the following: > - Do not assume that all devices support MMAP style buffer access. > - Do not assume that the hardware pointer inside the (possibly mmaped) > playback buffer is the actual position of the sample in the DAC. There > might be an extra latency involved. > - Do not try to recover with your own code from ALSA error conditions > such as buffer under-runs. Use snd_pcm_recover() instead. > - Do not touch buffering/period metrics unless you have specific > latency needs. Develop defensively, handling correctly the case when > the backend cannot fulfill your buffering metrics requests. Be aware > that the buffering metrics of the playback buffer only indirectly > influence the overall latency in many cases. i.e. setting the buffer > size to a fixed value might actually result in practical latencies > that are much higher. > > Especially the first one direct sound is all about direct card access > and hence mmap .. > > Roderick > It would be very nice if we could meet PulseAudio at least half way here. So far we've essentially made a massive feature request for them: "do everything ALSA does" -- it would be much nicer (and much more likely to happen) if we could distill that down to just a few of the above points that are essential with good explanations as to why we have to do it that way. Thanks, Scott Ritchie

June 2, 2009

Re: Website: Point the Development pages entirely to the Wiki.

by Scott Ritchie

Jeremy Newman wrote: > I'm holding this patch hostage until the Wiki is updated with the new > theme. > > -Newman > > Jeremy White wrote: >> --- >> templates/en/home.template | 4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/templates/en/home.template b/templates/en/home.template >> index bad2f62..870a934 100644 >> --- a/templates/en/home.template >> +++ b/templates/en/home.template >> @@ -24,8 +24,8 @@ >> <div class="subtitle">Documentation, Support, and >> Wiki.</div> >> </li> >> >> - <li class="devel" >> onClick="document.location='{$root}/devel/';"> >> - <div class="title"><a >> href="{$root}/devel/">Development</a></div> >> + <li class="devel" >> onClick="document.location='http://wiki.winehq.org/Developers';"> >> + <div class="title"><a >> href="http://wiki.winehq.org/Developers">Development</a></div> >> <div class="subtitle">Wine source, Bug Tracking, etc.</div> >> </li> >> >> >> > > Aren't you the one who's updating the wiki to the theme? Thanks, Scott Ritchie

June 2, 2009