13

Trying to debug a problem on a C# application, I stumbled upond this problem which is the cause of the app malfunctioning.

Basically I have this code:

double scale = 1;
double startScale = 1;
...
scale = (e.Scale - 1) * startScale;
if(scale <= 1)
    scale = 1;
...

What happens is that even if scale is greater than 1 the excecution enters inside the if the scale ends up being Always 1.

This happens only in release build.

Does anyone have an idea of what's going on?

EDIT

This is the, almost (missing only the ctor which does nothing, of a custom control for Xamarin Forms, taken from their example to implement a pinch gesture (here).

public class PinchView : ContentView
{
    private double StartScale = 1;
    private double CurrentScale = 1;
    private double XOffset = 0;
    private double YOffset = 0;

    ...

    private void PinchGesture_PinchUpdated(object sender, PinchGestureUpdatedEventArgs e)
    {
        if (e.Status == GestureStatus.Started)
        {
            // Store the current scale factor applied to the wrapped user interface element,
            // and zero the components for the center point of the translate transform.
            StartScale = Content.Scale;
            Content.AnchorX = 0;
            Content.AnchorY = 0;
        }

        if (e.Status == GestureStatus.Running)
        {
            // Calculate the scale factor to be applied.
            CurrentScale += (e.Scale - 1) * StartScale;
            if(CurrentScale <= 1)
            {
                CurrentScale = 1;
            }

            // The ScaleOrigin is in relative coordinates to the wrapped user interface element,
            // so get the X pixel coordinate.
            double renderedX = Content.X + XOffset;
            double deltaX = renderedX / Width;
            double deltaWidth = Width / (Content.Width * StartScale);
            double originX = (e.ScaleOrigin.X - deltaX) * deltaWidth;

            // The ScaleOrigin is in relative coordinates to the wrapped user interface element,
            // so get the Y pixel coordinate.
            double renderedY = Content.Y + YOffset;
            double deltaY = renderedY / Height;
            double deltaHeight = Height / (Content.Height * StartScale);
            double originY = (e.ScaleOrigin.Y - deltaY) * deltaHeight;

            // Calculate the transformed element pixel coordinates.
            double targetX = XOffset - (originX * Content.Width) * (CurrentScale - StartScale);
            double targetY = YOffset - (originY * Content.Height) * (CurrentScale - StartScale);

            // Apply translation based on the change in origin.
            Content.TranslationX = targetX.Clamp(-Content.Width * (CurrentScale - 1), 0);
            Content.TranslationY = targetY.Clamp(-Content.Height * (CurrentScale - 1), 0);

            // Apply scale factor.
            Content.Scale = CurrentScale;
        }

        if (e.Status == GestureStatus.Completed)
        {
            // Store the translation delta's of the wrapped user interface element.
            XOffset = Content.TranslationX;
            YOffset = Content.TranslationY;
        }
    }
}

These are steps of my debug session (e.Scale has been optimized and isn't visible, but you can see the value of CurrentScale changing):

enter image description here

enter image description here

enter image description here

enter image description here

enter image description here

Monish Koyott
  • 374
  • 1
  • 4
  • 20
zeb
  • 1,105
  • 2
  • 9
  • 32
  • 6
    We need more info. How dow you know `scale` is greater than one? Where does `e.Scale` come from? – Pikoh Oct 02 '17 at 08:28
  • 2
    Can you show us an example of the bug, with actual values ? Maybe a screenshot of a debugging session ? What is the value and type of each of the variables in your example when you put a breakpoint on the if statement ? – Mathieu VIALES Oct 02 '17 at 08:29
  • Yes, we need more code. This looks like an optimization result, but we need to see as much code as possible. – piojo Oct 02 '17 at 08:29
  • Do you have any #if DEBUG configured anywhere related to this logic? – Vijayanath Viswanathan Oct 02 '17 at 08:30
  • 3
    nitpicking - change `<=` in the condition to `<`. No point of assigning 1 to a variable that's already equal to 1. – Zohar Peled Oct 02 '17 at 08:30
  • 4
    Related or duplicate: [Why is floating point arithmetic in C# imprecise?](https://stackoverflow.com/q/753948/993547) – Patrick Hofman Oct 02 '17 at 08:31
  • @ZoharPeled yeah that's right, it was for testing purposes. BTW, I'll add more code – zeb Oct 02 '17 at 08:31
  • If it's a rounding error, you might be able to fix it by changing the condition to something like `if (scale <= 1.00001)` - but whether this is the right thing to do really depends on context. – Matthew Watson Oct 02 '17 at 08:46
  • And if it's not a rounding error, do you compile for AnyCPU and does anything change if you tick "Prefer 32-bit" or select x86 altogether? – GSerg Oct 02 '17 at 08:47
  • 1
    @MatthewWatson it's not a rounding problems as you can see on the new pictures I added. One thing to note though is that testing this on my physical device (Windows 10 Mobile with ARM processor) the error happens, tensing on the emulation with x86 the error doesn't exists. – zeb Oct 02 '17 at 08:49
  • That's weird indeed. Can you try with `if (CurrentScale<=1.0)` for example? – Pikoh Oct 02 '17 at 08:57
  • @Pikoh just tried. Doesn't change anything – zeb Oct 02 '17 at 09:01
  • There has been a fair amount of questions highlighting [x64 JIT bugs](https://www.google.com/search?q=c%23+jit+optimizer+bug+site:stackoverflow.com) that do not happen under x86. – GSerg Oct 02 '17 at 09:03
  • Do you have any way to log things as your program runs? ie output to file or similar the values of `CurrentScale` and `CurrentScale < 1` to check for sure what is going on there... – Chris Oct 02 '17 at 09:05
  • 2
    @Chris looking into it – zeb Oct 02 '17 at 09:14
  • 3
    @Chris doing tests to write the log file I had to change the callback for the pinch event to be `async` and I noticed that it started working. Then I removed the calls to the log method I made because I thought that maybe something wasnit getting optimized, but it turned out that only by making the callback an `async` method the problem is solved. Don't know why... – zeb Oct 02 '17 at 10:22
  • 1
    Well I imagine the difference between syn and async is probably pretty big in the optimizer so it still probably is that. I've no idea how you'd fix that or is being async actually acceptable to your application? – Chris Oct 02 '17 at 10:25
  • 2
    Yeah well, in this case I don't mind making the method async, it's still runner as sync since it doesn't contain any await. Do you know if there is a tool to see how C# optimizes code? To see the differences because this could be a bug for the ARM processors running C# no? – zeb Oct 02 '17 at 10:35
  • I'm not sure how to analyze the code after it has gone through the optimizer, no. :( – Chris Oct 02 '17 at 10:46
  • 2
    No problems, thanks :) – zeb Oct 02 '17 at 10:56
  • It looks like your code is almost verbatim:https://github.com/xamarin/xamarin-forms-samples/blob/master/WorkingWithGestures/PinchGesture/PinchGesture/PinchToZoomContainer.cs . Not sure if this will help, but it appears that you're using a callback event handler. Maybe this is causing your event handler to run on another thread and you are getting thread interference? Not sure how your code is being called outside of what you've shown but: https://developer.xamarin.com/guides/ios/user_interface/ios-ui/ui-thread/ may help? – Alexander Toptygin Oct 06 '17 at 19:10
  • Yes, my code is basically verbatim, I just added the pan to handle that too. Anyway, I don't really know at this point if this is caused by thread interference or from an optimization problem, but adding the async keywork to the method does the trick – zeb Oct 07 '17 at 06:30
  • @zeb I remember a problem I faced like this a very long time ago using Visual Studio. I found out that it was caused by multiple semi columns at the line endings of my code. It took me forever to discover and I faced similar issues to what you were describing. Do a search for double semi columns ;; and see if it pops up any results. – Peter H Nov 22 '17 at 06:07

1 Answers1

1

CurrentScale and StartScale are instance fields and not method fields, so they can be affected by another thread. To detect any race condition, can you put a lock around this bloc and debug again:

lock (StartScale) {
    // Calculate the scale factor to be applied.
    CurrentScale += (e.Scale - 1) * StartScale;
    if(CurrentScale <= 1)
    {
        CurrentScale = 1;
    }
}

If this works, you should have better results with local variables to reduce external interferences (and so remove the lock):

lock (StartScale) {
    // Calculate the scale factor to be applied.
    var localCurrentScale = CurrentScale + (e.Scale - 1) * StartScale;
    if(localCurrentScale <= 1)
    {
        CurrentScale = 1;
    } else CurrentScale = localCurrentScale;
}

You can use tools like Redgate Reflector or Telerik JustDecompile (free) to analyze your compiled assembly (it's pretty readable) and see what's been optimized during the compilation.

N.B.: StartScale is a bad lock but is fine for this debugging.

Veovis
  • 151
  • 1
  • 6
  • 2
    Nobody can put that into a comment. Size restrictions apply to everybody. Why do you want to? Consider changing the phrasing, please. The way this starts off currently gives the first impression that this is not an answer. – Yunnosch Dec 20 '17 at 08:08
  • well, you right, this is a pretty big "comment", however I doubt this is "the" answer but it's a procedure to give more informations on what's wrong except if by chance I'm right about the concurrency. Thanks for the suggestion @Yunnosch. – Veovis Dec 20 '17 at 10:55
  • Answers which provide some insight into a yet unsolved problem are appreciated at StackOverflow, even if they do not solve everything right away. So do not worry. You could be outspoken about this, e.g. by opening with some "In order to provide at least some partial help...". But in my opinion not even that is really necessary. – Yunnosch Dec 20 '17 at 11:15