Re: gdi32: Improve detection of symbol charset for old truetype fonts.
On Wed, Nov 18, 2015 at 02:16:36PM +0800, Dmitry Timoshkov wrote:
This patch uses same check for symbol charset that get_outline_text_metrics() already does, and allows the font attached to the bug 33117 be correctly recognized as a symbol ttf (like Windows does).
I think this needs to wait until after 1.8. It's probably correct, but there's enough doubt to not risk it (and yes, I know it's tricky to write a convincing test). My concern is that although it works for the font mentioned in the bug, there may well be other fonts out there that are affected in undesirable ways. Huw.
Huw Davies <huw(a)codeweavers.com> wrote:
On Wed, Nov 18, 2015 at 02:16:36PM +0800, Dmitry Timoshkov wrote:
This patch uses same check for symbol charset that get_outline_text_metrics() already does, and allows the font attached to the bug 33117 be correctly recognized as a symbol ttf (like Windows does).
I think this needs to wait until after 1.8. It's probably correct, but there's enough doubt to not risk it (and yes, I know it's tricky to write a convincing test). My concern is that although it works for the font mentioned in the bug, there may well be other fonts out there that are affected in undesirable ways.
If even so simple one-liners are considered risky, I can't think of any reasonable fix that could be included without a risk to break something during code freeze, not mentioning the regressions that sit in the bugzilla for ages without anyone even looking at them. Probably it's not worth spending time on trying to fix something for 1.8 with such an attitude. -- Dmitry.
On Wed, Nov 18, 2015 at 05:53:20PM +0800, Dmitry Timoshkov wrote:
Huw Davies <huw(a)codeweavers.com> wrote:
On Wed, Nov 18, 2015 at 02:16:36PM +0800, Dmitry Timoshkov wrote:
This patch uses same check for symbol charset that get_outline_text_metrics() already does, and allows the font attached to the bug 33117 be correctly recognized as a symbol ttf (like Windows does).
I think this needs to wait until after 1.8. It's probably correct, but there's enough doubt to not risk it (and yes, I know it's tricky to write a convincing test). My concern is that although it works for the font mentioned in the bug, there may well be other fonts out there that are affected in undesirable ways.
If even so simple one-liners are considered risky, I can't think of any reasonable fix that could be included without a risk to break something during code freeze, not mentioning the regressions that sit in the bugzilla for ages without anyone even looking at them. Probably it's not worth spending time on trying to fix something for 1.8 with such an attitude.
Not at all. The point is that this particular one-liner is most certainly not simple, for reasons that I tried to outline. Huw.
Huw Davies <huw(a)codeweavers.com> wrote:
On Wed, Nov 18, 2015 at 02:16:36PM +0800, Dmitry Timoshkov wrote:
This patch uses same check for symbol charset that get_outline_text_metrics() already does, and allows the font attached to the bug 33117 be correctly recognized as a symbol ttf (like Windows does).
I think this needs to wait until after 1.8. It's probably correct, but there's enough doubt to not risk it (and yes, I know it's tricky to write a convincing test). My concern is that although it works for the font mentioned in the bug, there may well be other fonts out there that are affected in undesirable ways.
If even so simple one-liners are considered risky, I can't think of any reasonable fix that could be included without a risk to break something during code freeze, not mentioning the regressions that sit in the bugzilla for ages without anyone even looking at them. Probably it's not worth spending time on trying to fix something for 1.8 with such an attitude.
Not at all. The point is that this particular one-liner is most certainly not simple, for reasons that I tried to outline.
And that's true for any other patch regardless of the area and amount of code. Basically any patch can have side effects, but if even in the first week of code-freeze any patch is considered as risky, then I don't know how you are going to fix the regressions or any bugs during that time. -- Dmitry.
On Wed, Nov 18, 2015 at 06:31:21PM +0800, Dmitry Timoshkov wrote:
Huw Davies <huw(a)codeweavers.com> wrote:
Not at all. The point is that this particular one-liner is most certainly not simple, for reasons that I tried to outline.
And that's true for any other patch regardless of the area and amount of code. Basically any patch can have side effects, but if even in the first week of code-freeze any patch is considered as risky, then I don't know how you are going to fix the regressions or any bugs during that time.
It's a risk-benefit decision, and that decision is by its very nature somewhat subjective. This patch potentially risks mishandling many OS/2 version-0 fonts that have worked up until now. I'm sure you can see that. Writing a test would of course reduce the risk, but as I said, I know that doing so is tricky. Huw.
Huw Davies <huw(a)codeweavers.com> wrote:
Not at all. The point is that this particular one-liner is most certainly not simple, for reasons that I tried to outline.
And that's true for any other patch regardless of the area and amount of code. Basically any patch can have side effects, but if even in the first week of code-freeze any patch is considered as risky, then I don't know how you are going to fix the regressions or any bugs during that time.
It's a risk-benefit decision, and that decision is by its very nature somewhat subjective. This patch potentially risks mishandling many OS/2 version-0 fonts that have worked up until now. I'm sure you can see that. Writing a test would of course reduce the risk, but as I said, I know that doing so is tricky.
Be my guest, and try to formulate similar excuses for every other patch you see in wine-patches during the code freeze. You are doing pretty well :) -- Dmitry.
On Wed, Nov 18, 2015 at 06:55:50PM +0800, Dmitry Timoshkov wrote:
Be my guest, and try to formulate similar excuses for every other patch you see in wine-patches during the code freeze. You are doing pretty well :)
I'm not doing it for fun. It would have been much easier for me to sign them off. Huw.
Huw Davies <huw(a)codeweavers.com> wrote:
On Wed, Nov 18, 2015 at 06:55:50PM +0800, Dmitry Timoshkov wrote:
Be my guest, and try to formulate similar excuses for every other patch you see in wine-patches during the code freeze. You are doing pretty well :)
I'm not doing it for fun. It would have been much easier for me to sign them off.
Oh, so if a signing off requirement actually leads to this kind of fears, then I'd say that you probably shouldn't pretend to be a maintainer. -- Dmitry.
On Wed, Nov 18, 2015 at 07:24:27PM +0800, Dmitry Timoshkov wrote:
Oh, so if a signing off requirement actually leads to this kind of fears, then I'd say that you probably shouldn't pretend to be a maintainer.
[Let's not get personal here if you don't mind.] There's no point in having maintainers if they sign off every patch. I've explained why I think this patch is risky; I'm surprised that you don't see that risk. At the end of the day it's AJ's decision, he's free to commit this without my sign-off. Huw.
Huw Davies <huw(a)codeweavers.com> wrote:
On Wed, Nov 18, 2015 at 07:24:27PM +0800, Dmitry Timoshkov wrote:
Oh, so if a signing off requirement actually leads to this kind of fears, then I'd say that you probably shouldn't pretend to be a maintainer.
[Let's not get personal here if you don't mind.]
I'm sorry if my sentence above sounded rude, I haven't really meant that.
There's no point in having maintainers if they sign off every patch.
I've explained why I think this patch is risky; I'm surprised that you don't see that risk.
How many fonts with OS/2 version 0 are out there and especially the ones that are supposed to have symbol charmap and don't? I've run a simple app that enumerates the fonts under Wine and Windows and I have 0 (zero) such fonts.
At the end of the day it's AJ's decision, he's free to commit this without my sign-off.
I'm sure that you know that it could happen only if the sender has e-mail address ending with @codeweavers.com, statistics of last two months is pretty clear about that. -- Dmitry.
Dmitry Timoshkov <dmitry(a)baikal.ru> writes:
How many fonts with OS/2 version 0 are out there and especially the ones that are supposed to have symbol charmap and don't? I've run a simple app that enumerates the fonts under Wine and Windows and I have 0 (zero) such fonts.
So how many apps out there require your fix? And which ones? -- Alexandre Julliard julliard(a)winehq.org
Am Mittwoch, 18. November 2015 schrieb Alexandre Julliard:
So how many apps out there require your fix? And which ones?
One example I'm familiar with is https://en.wikipedia.org/wiki/Capella_%28notation_program%29, which is unusable (as in no music notes are displayed) without this fix.
On Wed, Nov 18, 2015 at 6:08 AM, Dmitry Timoshkov <dmitry(a)baikal.ru> wrote:
Huw Davies <huw(a)codeweavers.com> wrote:
On Wed, Nov 18, 2015 at 07:24:27PM +0800, Dmitry Timoshkov wrote:
Oh, so if a signing off requirement actually leads to this kind of fears, then I'd say that you probably shouldn't pretend to be a maintainer.
[Let's not get personal here if you don't mind.]
I'm sorry if my sentence above sounded rude, I haven't really meant that.
There's no point in having maintainers if they sign off every patch.
I've explained why I think this patch is risky; I'm surprised that you don't see that risk.
How many fonts with OS/2 version 0 are out there and especially the ones that are supposed to have symbol charmap and don't? I've run a simple app that enumerates the fonts under Wine and Windows and I have 0 (zero) such fonts.
Is that application available publicly somewhere? -- -Austin
On 18.11.2015 11:31, Dmitry Timoshkov wrote:
And that's true for any other patch regardless of the area and amount of code. Basically any patch can have side effects, but if even in the first week of code-freeze any patch is considered as risky, then I don't know how you are going to fix the regressions or any bugs during that time.
I do not want to heat up the discussion, but this is indeed a valid concern, and its also a bit unclear to me what exactly is allowed during the code freeze. In theory, almost every commit could cause regressions. When adding new stubs applications might use a different code paths. When fixing one issue, it might reveal a different one somewhere else. And especially regarding the remaining regressions: those which already exist for a long time often aren't that easy to fix and also might require reworking a lot of the existing code. This means the risk of adding new regressions is quite high, when they are applied without any testing. Imho, the only guarantee to ensure a really stable version, would be to develop on both "stable" and "development" in parallel, and frequently backport patches which have been tested well enough - basically similar to our Staging tree. Unfortunately this doesn't force anyone to work on regressions ... ;) So, what kind of patches are acceptable during the freeze period? If we are too strict, basically only minor cleanup is possible. I still have various patches I would like to send (x86_64 marshalling, ...), but a small risk of regressions is always there.
On 18.11.2015 19:07, Sebastian Lackner wrote:
On 18.11.2015 11:31, Dmitry Timoshkov wrote:
And that's true for any other patch regardless of the area and amount of code. Basically any patch can have side effects, but if even in the first week of code-freeze any patch is considered as risky, then I don't know how you are going to fix the regressions or any bugs during that time.
I do not want to heat up the discussion, but this is indeed a valid concern, and its also a bit unclear to me what exactly is allowed during the code freeze. In theory, almost every commit could cause regressions. When adding new stubs applications might use a different code paths. When fixing one issue, it might reveal a different one somewhere else.
Of course any commit could cause a problem sooner or later, the question is which parts are affected. In case of gdi32 virtually every application is.
And especially regarding the remaining regressions: those which already exist for a long time often aren't that easy to fix and also might require reworking a lot of the existing code. This means the risk of adding new regressions is quite high, when they are applied without any testing.
Such regressions should be fixed long before release, not in last couple of weeks.
Imho, the only guarantee to ensure a really stable version, would be to develop on both "stable" and "development" in parallel, and frequently backport patches which have been tested well enough - basically similar to our Staging tree. Unfortunately this doesn't force anyone to work on regressions ... ;)
That was discussed, you were there. Stable as we know it stays, with frequent backports that are critical, also translation updates, maybe something else.
So, what kind of patches are acceptable during the freeze period?
The ones that definitely work? Come with test? Don't depend on user system state that is hard to replicate, like any possible set of installed fonts. I don't really understand a drama about holding a patch for another month, and sending it again after release.
Sebastian Lackner <sebastian(a)fds-team.de> writes:
So, what kind of patches are acceptable during the freeze period? If we are too strict, basically only minor cleanup is possible. I still have various patches I would like to send (x86_64 marshalling, ...), but a small risk of regressions is always there.
Acceptable patches are the ones where the risk of adding a regression is considered smaller than the benefit that the patch provides. That's a subjective judgment based on past experience, so there's no hard and fast rule. But if the patch is more than a few lines, or touches an area that's known to be prone to regressions, it's not likely to go in. I'm also willing to be convinced, so if you can explain why you think that the patch is safe, or why you think the benefits are big enough, that helps. But of course, because it's subjective, there will always be cases where we have to agree to disagree. Code freeze is not about achieving perfection, it's about best effort. Even if a patch doesn't get into 1.8, the important thing is that we have taken the time to develop a fix. If that fix turns out to be too risky, that's not the end of the world, it will go into the development branch, and if it turns out that it was safe after all, it can then be backported to 1.8.x. -- Alexandre Julliard julliard(a)winehq.org
participants (7)
-
Alexandre Julliard -
Austin English -
Dmitry Timoshkov -
Huw Davies -
Joachim Priesner -
Nikolay Sivov -
Sebastian Lackner