Re: [Geotools-devel] Fix for ImageWorker
Hi Andrea, Last time I tried I got the "license exceeded for geotools" error from Jira/Atlassian. Today, when I try to login, I get a parachute, with text "Joining Jira. Hang on a sec...", which is later appended by a "try again" button. I tried that three times, all the same affect. First going to OSgeo (left upper home button) I get a localized message stating that "our team is notified, if the problem persist, please contact Atlassian support". This is the code that attached to that bubble text: "ERROR: 18VOQVT". Reloading the same thing. I get the the list of issues, but the left frame remains blank with an "error" road sign. I have no trouble logging into atlassian root, I have an account. (Unless those are different realms - but then I don't know how to apply for an account for geotools.) The account is keyed to this email address. Sorry. Michael On Wed, Dec 01, 2021 at 06:58:38PM +0100, Andrea Aime wrote: > Hi Michael, > anyone should be able to create a Jira ticket, given an account (which can > also be freely created). > Did you create the account and it still does not work? > > Cheers > Andrea > > On Mon, Nov 29, 2021 at 7:01 PM Michael Jung > wrote: > > > Issued a pull request. I couldn't issue a Jira issue accordingly, license > > issues it says. > > > > 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
Re: [Geotools-devel] Fix for ImageWorker
Hi Michael,
anyone should be able to create a Jira ticket, given an account (which can
also be freely created).
Did you create the account and it still does not work?
Cheers
Andrea
On Mon, Nov 29, 2021 at 7:01 PM Michael Jung
wrote:
> Issued a pull request. I couldn't issue a Jira issue accordingly, license
> issues it says.
>
> 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),
> > > > > > > -
> layout.getTileGridYOffset(null),
> > > > > > > -layout.getTileWidth(null),
> > > > > > > -layout.getTileHeight(null),
> > > >
Re: [Geotools-devel] Fix for ImageWorker
Issued a pull request. I couldn't issue a Jira issue accordingly, license
issues it says.
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),
> > > > > > -layout.getTileGridYOffset(null),
> > > > > > -layout.getTileWidth(null),
> > > > > > -layout.getTileHeight(null),
> > > > > > -sm,
> > > > > > -cm);
> > > > > > -merged.setRenderingHints(new
> > > > > > RenderingHints(JAI.KEY_IMAGE_LAYOUT, layout2));
> > > > > > +if (layout.getTileWidth(null) > 0 &&
> > > > > > layout.getTileHeight(null) > 0) {
> > > > > > +ImageLayout layout2 =
> > > > > > +new ImageLayout2(
> > > > > > +
>
Re: [Geotools-devel] Fix for ImageWorker
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),
> > > >
Re: [Geotools-devel] Fix for ImageWorker
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),
> > > > > -layout.getTileGridYOffset(null),
> > > > > -layout.getTileWidth(null),
> > > > > -layout.getTileHeight(null),
> > > > > -sm,
> > > > > -cm);
> > > > > -merged.setRenderingHints(new
> > > > > RenderingHints(JAI.KEY_IMAGE_LAYOUT, layout2));
> > > > > +if (layout.getTileWidth(null) > 0 &&
> > > > > layout.getTileHeight(null) > 0) {
> > > > > +ImageLayout layout2 =
> > > > > +new ImageLayout2(
> > > > > +
> layout.getTileGridXOffset(null),
> > > > > +
> layout.getTileGridYOffset(null),
> > > > > +layout.getTileWidth(null),
> > > > > +layout.getTileHeight(null),
> > > > > +sm,
> > > > > +cm);
> > > > > +merged.setRenderingHints(new
> > >
Re: [Geotools-devel] Fix for ImageWorker
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
> > > 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),
> > > > -layout.getTileGridYOffset(null),
> > > > -layout.getTileWidth(null),
> > > > -layout.getTileHeight(null),
> > > > -sm,
> > > > -cm);
> > > > -merged.setRenderingHints(new
> > > > RenderingHints(JAI.KEY_IMAGE_LAYOUT, layout2));
> > > > +if (layout.getTileWidth(null) > 0 &&
> > > > layout.getTileHeight(null) > 0) {
> > > > +ImageLayout layout2 =
> > > > +new ImageLayout2(
> > > > +layout.getTileGridXOffset(null),
> > > > +layout.getTileGridYOffset(null),
> > > > +layout.getTileWidth(null),
> > > > +layout.getTileHeight(null),
> > > > +sm,
> > > > +cm);
> > > > +merged.setRenderingHints(new
> > > > RenderingHints(JAI.KEY_IMAGE_LAYOUT, layout2));
> > > > +}
> > > > }
> > > > image = merged.addBand(alphaChannel, false, true,
> > > > null).getRenderedImage();
> > > > }
> > > >
> > > > I could also create a pull request (from
> > > > https://github.com/irisiflimsi/geotools) and an issue for it, if you
> > > > prefer.
> > > >
> > > > Sincerely,
> > > >
Re: [Geotools-devel] Fix for ImageWorker
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
> > 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),
> > > -layout.getTileGridYOffset(null),
> > > -layout.getTileWidth(null),
> > > -layout.getTileHeight(null),
> > > -sm,
> > > -cm);
> > > -merged.setRenderingHints(new
> > > RenderingHints(JAI.KEY_IMAGE_LAYOUT, layout2));
> > > +if (layout.getTileWidth(null) > 0 &&
> > > layout.getTileHeight(null) > 0) {
> > > +ImageLayout layout2 =
> > > +new ImageLayout2(
> > > +layout.getTileGridXOffset(null),
> > > +layout.getTileGridYOffset(null),
> > > +layout.getTileWidth(null),
> > > +layout.getTileHeight(null),
> > > +sm,
> > > +cm);
> > > +merged.setRenderingHints(new
> > > RenderingHints(JAI.KEY_IMAGE_LAYOUT, layout2));
> > > +}
> > > }
> > > image = merged.addBand(alphaChannel, false, true,
> > > null).getRenderedImage();
> > > }
> > >
> > > I could also create a pull request (from
> > > https://github.com/irisiflimsi/geotools) and an issue for it, if you
> > > prefer.
> > >
> > > Sincerely,
> > > Michael
> > >
> > >
> > >
> > > ___
> > > GeoTools-Devel mailing list
> > > GeoTools-Devel@lists.sourceforge.net
> > > https://lists.sourceforge.net/lists/listinfo/geotools-devel
> > >
> > --
> > --
> > Jody Garnett
>
___
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel
Re: [Geotools-devel] Fix for ImageWorker
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
> 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),
> > -layout.getTileGridYOffset(null),
> > -layout.getTileWidth(null),
> > -layout.getTileHeight(null),
> > -sm,
> > -cm);
> > -merged.setRenderingHints(new
> > RenderingHints(JAI.KEY_IMAGE_LAYOUT, layout2));
> > +if (layout.getTileWidth(null) > 0 &&
> > layout.getTileHeight(null) > 0) {
> > +ImageLayout layout2 =
> > +new ImageLayout2(
> > +layout.getTileGridXOffset(null),
> > +layout.getTileGridYOffset(null),
> > +layout.getTileWidth(null),
> > +layout.getTileHeight(null),
> > +sm,
> > +cm);
> > +merged.setRenderingHints(new
> > RenderingHints(JAI.KEY_IMAGE_LAYOUT, layout2));
> > +}
> > }
> > image = merged.addBand(alphaChannel, false, true,
> > null).getRenderedImage();
> > }
> >
> > I could also create a pull request (from
> > https://github.com/irisiflimsi/geotools) and an issue for it, if you
> > prefer.
> >
> > Sincerely,
> > Michael
> >
> >
> >
> > ___
> > GeoTools-Devel mailing list
> > GeoTools-Devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/geotools-devel
> >
> --
> --
> Jody Garnett
___
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel
Re: [Geotools-devel] Fix for ImageWorker
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
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),
> -layout.getTileGridYOffset(null),
> -layout.getTileWidth(null),
> -layout.getTileHeight(null),
> -sm,
> -cm);
> -merged.setRenderingHints(new
> RenderingHints(JAI.KEY_IMAGE_LAYOUT, layout2));
> +if (layout.getTileWidth(null) > 0 &&
> layout.getTileHeight(null) > 0) {
> +ImageLayout layout2 =
> +new ImageLayout2(
> +layout.getTileGridXOffset(null),
> +layout.getTileGridYOffset(null),
> +layout.getTileWidth(null),
> +layout.getTileHeight(null),
> +sm,
> +cm);
> +merged.setRenderingHints(new
> RenderingHints(JAI.KEY_IMAGE_LAYOUT, layout2));
> +}
> }
> image = merged.addBand(alphaChannel, false, true,
> null).getRenderedImage();
> }
>
> I could also create a pull request (from
> https://github.com/irisiflimsi/geotools) and an issue for it, if you
> prefer.
>
> Sincerely,
> Michael
>
>
>
> ___
> GeoTools-Devel mailing list
> GeoTools-Devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>
--
--
Jody Garnett
___
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel
