I have one testcase now that verifies the patch. (The private method
now calls a package method prepareForScaledAlphaChannel.)
+@Test(expected = Test.None.class /* No IllegalArgumentException */)
+public void testReattachScaledAlphaChannel() {
+BufferedImage bi = new BufferedImage(1, 1,
BufferedImage.TYPE_BYTE_GRAY);
+ImageWorker iw = new ImageWorker(bi);
+RenderingHints hints = new RenderingHints(null);
+hints.put(JAI.KEY_IMAGE_LAYOUT, new ImageLayout(0, 0, 1, 1));
+iw.prepareForScaledAlphaChannel(bi, hints, bi.getColorModel(),
bi.getSampleModel());
+}
Should I prepare a PR and would you want the full program (issue, branch of the
same name) or should I use my main and add some comments in the PR?
Michael
On Tue, Oct 19, 2021 at 09:05:42AM -0700, Jody Garnett wrote:
> Not really, you can also make such a method package visible (which
> depending on your viewpoint on encapsulation is slightly less visible than
> protected).
> --
> Jody Garnett
>
>
> On Mon, 18 Oct 2021 at 11:55, Michael Jung wrote:
>
> > I guess hanging out means waiting here?
> > I think, I can extract another private method, turn it protected and test
> > it. (Is there a better approach to test private methods these days?)
> >
> > Michael
> >
> > On Fri, Oct 15, 2021 at 05:11:46PM -0700, Jody Garnett wrote:
> > > I am not that familiar with image workers myself, I think we should hang
> > > out for a bit and ask for help in how to write test cases.
> > > --
> > > Jody Garnett
> > >
> > >
> > > On Fri, 15 Oct 2021 at 03:39, Michael Jung
> > wrote:
> > >
> > > > I understand your policy, and I have tried complying. However, it is
> > > > not that simple.
> > > >
> > > > The scale method is deeply entrenched in the ImageWorker and testing
> > > > this safety check would require me to refactor a lot of code that
> > > > I am not familiar with - or to reverse engineer my "big"
> > > > example until I find the parameter-set on class level that causes the
> > > > problem. The first has the danger of introducing new bugs and the
> > latter
> > > > is hideously difficult, as you may imagine. At least it is not worth
> > > > the effort for such an unproblematic safety check.
> > > >
> > > > I'm sorry that this is beyond my ability. Would you still take a pull
> > > > request?
> > > >
> > > > Michael
> > > >
> > > > On Thu, Oct 14, 2021 at 12:18:10PM -0700, Jody Garnett wrote:
> > > > > Thanks for contacting us, it appears your fix is good safety check.
> > > > >
> > > > > We do have the policy of only accepting pull requests with a test
> > case;
> > > > to
> > > > > prevent regressions from occurring and prove to the review that the
> > > > change
> > > > > addresses the issue in questions. We have had too much technical debt
> > > > > accumulate before this policy.
> > > > >
> > > > > We would love if if you can make a PR.
> > > > >
> > > > > Jody
> > > > >
> > > > > On Thu, Oct 14, 2021 at 11:37 AM Michael Jung <
> > m...@golem.phantasia.org>
> > > > > wrote:
> > > > >
> > > > > > Hello,
> > > > > >
> > > > > > I sent a similar mail a couple of months ago. I didn't receive any
> > > > > > response,
> > > > > > but I had only recently subscribed, maybe it got lost in
> > translation.
> > > > > >
> > > > > > The CONTRIBUTING.md says, I should post here.
> > > > > >
> > > > > > A lot of my data is raster data with alpha-channel. About 2% fail
> > when
> > > > > > serving them through the tile server at some resolutions. I can't
> > > > > > produce a small, self-contained example, unfortunately. But I have
> > > > > > tracked it down and it seems the alpha-channel is rendered
> > separately
> > > > > > from the rest of the image and is "allergic" to 0-size tile-width.
> > > > > > This is the way I fixed it in 21.1 (verified in my environment).
> > > > > > If necessary, I can give you the stack trace from the logs, the
> > > > > > AWT class doesn't like 0.
> > > > > >
> > > > > > This is the patch on current main HEAD:
> > > > > >
> > > > > > diff --git
> > > > > >
> > > >
> > a/modules/library/coverage/src/main/java/org/geotools/image/ImageWorker.java
> > > > > >
> > > >
> > b/modules/library/coverage/src/main/java/org/geotools/image/ImageWorker.java
> > > > > > index 0dfee3e..82c0d3f 100644
> > > > > > ---
> > > > > >
> > > >
> > a/modules/library/coverage/src/main/java/org/geotools/image/ImageWorker.java
> > > > > > +++
> > > > > >
> > > >
> > b/modules/library/coverage/src/main/java/org/geotools/image/ImageWorker.java
> > > > > > @@ -4452,15 +4452,17 @@ public class ImageWorker {
> > > > > > Object candidate = hints.get(JAI.KEY_IMAGE_LAYOUT);
> > > > > > if (candidate instanceof ImageLayout) {
> > > > > > ImageLayout layout = (ImageLayout) candidate;
> > > > > > -ImageLayout layout2 =
> > > > > > -new ImageLayout2(
> > > > > > -layout.getTileGridXOffset(null),
> > > >
