search

Spelt / ZXing.Delphi

ZXing Barcode Scanning object Pascal Library for Delphi VCL and Delphi Firemonkey
Apache License 2.0
471 stars 206 forks source link

Delphi 11 compatibility #123

Closed anderbelluno closed 2 years ago

flrizzato commented 2 years ago

I've just tested with RAD 11 and everything is working fine for Android 11 and iOS 15.1

anderbelluno commented 2 years ago

The integer overflow error appears for me. This did not happen in Delphi 10.4.2

igorbastosib commented 2 years ago

@flrizzato the error was also raising on my environment, so I made a fork to fix it. Let's see if it is ok. Could you try it on iOS, please? https://github.com/igorbastosib/ZXing.Delphi I'll send a pull request to the original project as soon as you feedback me.

Thank you

flrizzato commented 2 years ago

I'm a little bit worried with the fact you both are seeing this issue but I can't reproduce it on my environment for both Android and iOS. Have you identified the potential cause in the library source code?

igorbastosib commented 2 years ago

@flrizzato unfortunately we are not the only ones who faced this issue. I've seen few Devs on Social Media complaining about it...

I'm not that technical to understand the "why", sorry, I just found where was raising the issue and 'fixed' it though:

Lib/Classes/Common/ZXing.Common.BitArrayImplementation.pas Function: function TBitArrayImplementation.GetBits, line with: currentBits := currentBits and (not((1 shl (from and $1F)) - 1)); AND two places on TBitArrayImplementation.InitLookup... I added the {$OVERFLOWCHECKS OFF} before and {$OVERFLOWCHECKS ON} after to 'fix' it

flrizzato commented 2 years ago

Are you testing in debug mode?

https://www.ideasawakened.com/post/overflow-and-range-checking-are-now-enabled-by-default-for-debug-builds

So, maybe the issue was present in 10.4 but not visible...

igorbastosib commented 2 years ago

hahahah To be honest Darian Miller sent me this article yesterday, so I haven't truly fixed anything at all. At least the code I added is gonna disable it even if the DEV doesn't read the article (which was my case and so many others).

flrizzato commented 2 years ago

This change affects only "debug mode", so building in "release mode" before deploying will have the same results. Anyway, the core issue deserves more investigation, I will try asap.

igorbastosib commented 2 years ago

Please, do so.

As said here I just changed 2 places (the whole code is different because I'm used to press CtrlD every single time... sorry

@flrizzato unfortunately we are not the only ones who faced this issue. I've seen few Devs on Social Media complaining about it...

I'm not that technical to understand the "why", sorry, I just found where was raising the issue and 'fixed' it though:

Lib/Classes/Common/ZXing.Common.BitArrayImplementation.pas Function: function TBitArrayImplementation.GetBits, line with: currentBits := currentBits and (not((1 shl (from and $1F)) - 1)); AND two places on TBitArrayImplementation.InitLookup... I added the {$OVERFLOWCHECKS OFF} before and {$OVERFLOWCHECKS ON} after to 'fix' it

igorbastosib commented 2 years ago

Just did a "Pull Request" 'fixing' this situation. Let's see when @Spelt is going to review it šŸ‘šŸ‘

Spelt commented 2 years ago

Thanks,

I hope I can do it this month. Iā€™m unfortunately a bit stressed for time. I hope I can take a look at it at the end of this month.

Op 5 dec. 2021 om 11:26 heeft Igor @.***> het volgende geschreven:

ļ»æ Just did a "Pull Request" 'fixing' this situation. Let's see when @Spelt is going to review it šŸ‘šŸ‘

ā€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

igorbastosib commented 2 years ago

No worries, you will notice there is no that much changes: I just added 4 lines turning OFF and ON the IntergerOverflow, I also added some changes on the Demo\aTestAPP, because some stuff has changed on D11. Take your time šŸ˜‰

Thank you, Igor de Bastos Costa

De: E Spelt @.> Enviado: segunda-feira, 6 de dezembro de 2021 18:24 Para: Spelt/ZXing.Delphi @.> Cc: Igor @.>; Comment @.> Assunto: Re: [Spelt/ZXing.Delphi] Delphi 11 compatibility (Issue #123)

Thanks,

I hope I can do it this month. Iā€™m unfortunately a bit stressed for time. I hope I can take a look at it at the end of this month.

Op 5 dec. 2021 om 11:26 heeft Igor @.***> het volgende geschreven:

ļ»æ Just did a "Pull Request" 'fixing' this situation. Let's see when @Spelt is going to review it šŸ‘šŸ‘

ā€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

ā€” You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/Spelt/ZXing.Delphi/issues/123#issuecomment-986989619, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACG2RAB7NINZDGAAP3QTKHLUPTWVTANCNFSM5GVKQNTA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

Spelt commented 2 years ago

I could not reproduce this. I have improved the source a bit for better protection. I also have {$OVERFLOWCHECKS OFF} added.