Define IsFloatEqual macro properly.

Define IsFloatEqual macro properly.

BUG=chromium:663253
Committed: https://pdfium.googlesource.com/pdfium/+/4f610efcf4e34779d27c5e822e639a984bab1c96

[Lei Zhang, 2016-11-09 02:04:41]

The CQ bit was checked by thestig@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-09 02:04:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Lei Zhang, 2016-11-09 02:22:13]

thestig@chromium.org changed reviewers:
+ dsinclair@chromium.org

[Lei Zhang, 2016-11-09 02:22:14]

Looking at
https://codereview.chromium.org/2187073005/diff/100001/fpdfsdk/fxedit/include...,
I suspect this will fix pdfwindow, but cause subtle differences in fxedit. :\

[commit-bot: I haz the power, 2016-11-09 02:23:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-09 02:23:01]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-11-09 02:28:19]

On 2016/11/09 02:22:14, Lei Zhang (slow) wrote:
> Looking at
>
https://codereview.chromium.org/2187073005/diff/100001/fpdfsdk/fxedit/include...,
> I suspect this will fix pdfwindow, but cause subtle differences in fxedit. :\

Such as:

-      } else if ((m_ptScrollPos.x > rcContent.right - rcPlate.Width() &&
!((m_ptScrollPos.x - rcContent.right - rcPlate.Width()) < 0.0001 &&
(m_ptScrollPos.x - rcContent.right - rcPlate.Width()) > -0.0001))) {
+      } else if (((m_ptScrollPos.x) > (rcContent.right - rcPlate.Width()) &&
!(((m_ptScrollPos.x) - (rcContent.right - rcPlate.Width())) < 0.0001 &&
((m_ptScrollPos.x) - (rcContent.right - rcPlate.Width())) > -0.0001))) {

If I change "rcContent.right - rcPlate.Width()" to "rcContent.right +
rcPlate.Width()" to restore "m_ptScrollPos.x - rcContent.right -
rcPlate.Width()", then "m_ptScrollPos.x > rcContent.right - rcPlate.Width()" in
front becomes "m_ptScrollPos.x > rcContent.right + rcPlate.Width()"

Basically we have to restore the FX_EDIT_IsFloat* macros, if our goal is restore
the exact behavior from before https://codereview.chromium.org/2187073005 . Do
you see another way around it?

[dsinclair, 2016-11-09 03:30:54]

On 2016/11/09 02:28:19, Lei Zhang (slow) wrote:
> On 2016/11/09 02:22:14, Lei Zhang (slow) wrote:
> > Looking at
> >
>
https://codereview.chromium.org/2187073005/diff/100001/fpdfsdk/fxedit/include...,
> > I suspect this will fix pdfwindow, but cause subtle differences in fxedit.
:\
> 
> Such as:
> 
> -      } else if ((m_ptScrollPos.x > rcContent.right - rcPlate.Width() &&
> !((m_ptScrollPos.x - rcContent.right - rcPlate.Width()) < 0.0001 &&
> (m_ptScrollPos.x - rcContent.right - rcPlate.Width()) > -0.0001))) {
> +      } else if (((m_ptScrollPos.x) > (rcContent.right - rcPlate.Width()) &&
> !(((m_ptScrollPos.x) - (rcContent.right - rcPlate.Width())) < 0.0001 &&
> ((m_ptScrollPos.x) - (rcContent.right - rcPlate.Width())) > -0.0001))) {
> 
> If I change "rcContent.right - rcPlate.Width()" to "rcContent.right +
> rcPlate.Width()" to restore "m_ptScrollPos.x - rcContent.right -
> rcPlate.Width()", then "m_ptScrollPos.x > rcContent.right - rcPlate.Width()"
in
> front becomes "m_ptScrollPos.x > rcContent.right + rcPlate.Width()"
> 
> Basically we have to restore the FX_EDIT_IsFloat* macros, if our goal is
restore
> the exact behavior from before https://codereview.chromium.org/2187073005 . Do
> you see another way around it?


If the previous behaviour was wrong, why do we want to restore it? Is the issue
that you don't know what they actually wanted?

[dsinclair, 2016-11-09 03:33:23]

https://codereview.chromium.org/2482373002/diff/1/core/fxcrt/fx_system.h
File core/fxcrt/fx_system.h (right):

https://codereview.chromium.org/2482373002/diff/1/core/fxcrt/fx_system.h#newc...
core/fxcrt/fx_system.h:79: #define IsFloatEqual(fa, fb) IsFloatZero((fa) - (fb))
Should these just be real methods instead of macros so we don't end up with this
issue?

[Lei Zhang, 2016-11-09 05:33:05]

On 2016/11/09 03:30:54, dsinclair wrote:
> If the previous behaviour was wrong, why do we want to restore it? Is the
issue
> that you don't know what they actually wanted?

I don't know which is correct. All I'm saying is twiddling fx_system.h won't
ever restore the old behavior. This CL does restore the old behavior for
pdfwindow, but at the same time changes the fxedit behavior.

[Lei Zhang, 2016-11-09 05:34:10]

https://codereview.chromium.org/2482373002/diff/1/core/fxcrt/fx_system.h
File core/fxcrt/fx_system.h (right):

https://codereview.chromium.org/2482373002/diff/1/core/fxcrt/fx_system.h#newc...
core/fxcrt/fx_system.h:79: #define IsFloatEqual(fa, fb) IsFloatZero((fa) - (fb))
On 2016/11/09 03:33:23, dsinclair wrote:
> Should these just be real methods instead of macros so we don't end up with
this
> issue?

We can, but the behavior of this CL won't change from patch set 1.

[dsinclair, 2016-11-09 06:11:54]

lgtm

As far as I understand, this is the correct way to write the macro. If there is
code that is depending on the old behaviour we can fix it when we find it.

[Lei Zhang, 2016-11-09 06:15:29]

On 2016/11/09 06:11:54, dsinclair wrote:
> lgtm
> 
> As far as I understand, this is the correct way to write the macro. If there
is
> code that is depending on the old behaviour we can fix it when we find it.

Let's see what happens...

[Lei Zhang, 2016-11-09 06:15:33]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2016-11-09 06:15:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-09 06:15:53]

Description was changed from

==========
Define IsFloatEqual macro properly.

BUG=chromium:663253
==========

to

==========
Define IsFloatEqual macro properly.

BUG=chromium:663253

Committed:
https://pdfium.googlesource.com/pdfium/+/4f610efcf4e34779d27c5e822e639a984bab...
==========

[commit-bot: I haz the power, 2016-11-09 06:15:54]

Committed patchset #1 (id:1) as
https://pdfium.googlesource.com/pdfium/+/4f610efcf4e34779d27c5e822e639a984bab...