The regression tests for this bug do not call the method directly. They
may exercise the function indirectly in some pipelines, but not all
pipelines will use this method (the current version of Pisces in OpenJDK
doesn't even use it until you integrate your other changes as far as I
know).
I'd write a regression test for this bug that is directly applicable to
the method being tested (find what values are being handed to the method
by these test cases and then just call Cubic.solveCubic directly with
those values and figure out if the answers are reasonable).
If you want to include these rendering tests as extra verification along
with your other changes, then that is fine.
Also, I think we might have a script that forceably checks the value of
the @bug tag and ensures that it is a legal bug database number, so
using a RedHat bug number won't work very well. Is there an existing
bug that this could be tagged with?
...jim
On 12/20/2010 9:36 AM, Denis Lila wrote:
Hi Jim.
Lines 1094-1096, they could also be NaN if any of the numerators were
also zero and these tests might fail (but only for the case of all of
them being zero I guess, otherwise one of the other divisions would
result in infinity). Are accidental infinities (caused by overflow
rather than d==0.0) really a problem for the code to handle?
I'm not sure if they're a problem, but I thought that case should have
been handled just for robustness. However, I've changed the test
to d==0 because testing for infinities should be done, but not there.
For example, if the constant term was huge and d==0.5 we could get
an infinity but that shouldn't really be handled as a quadratic
polynomial. I will deal better with these cases in a future webrev.
I just noticed that the code you are replacing actually used to refine
the roots so maybe you should do some of this magic.
I missed that in the original code. I changed it now.
Also, in the webrev you'll find five regression tests that I would like
to push to openjdk7. They test for various problems the rendering engine
used to have. They're all pretty simple and I would appreciate it if you
could take a quick look at them. They're in the same webrev as cc2d because
it was more convenient for me, but obviously when/if they're pushed they
will be a separate changeset.
One more thing: there is a regression test for the rendering engine
called TestNPE that I think is problematic because it doesn't
necessarily test the rendering engine. It just draws an antialiased
line, which could be handled in any number of ways, so it's not very
robust. In fact, after we're done with the parallelogram pipelines,
the code that used to throw the NPE won't even execute, making this
test useless. We should either discard it or change it to use the
rendering engine explicitly, like my tests do. What do you think?
Regards,
Denis.
