Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shape font #3890

Merged
merged 7 commits into from Jul 21, 2015
Merged

Shape font #3890

merged 7 commits into from Jul 21, 2015

Conversation

jburel
Copy link
Member

@jburel jburel commented Jun 19, 2015

In this PR, fix the problem reported on forum see https://trac.openmicroscopy.org.uk/ome/ticket/12936

To test this PR:

  • log as user-3.
  • Go to Project-user-3/archivedv

Test 1

  • Open the first image (id 9461) in the full viewer
  • Open the measurement tool
  • Check that the stroke width is displayed correctly i.e. red and width of 15 and 10

Test 2

  • Open the first image (id 9460) in the full viewer
  • Open the measurement tool
  • Check that no message pops up. Check that you can see the roi

Test 3

  • Open the first image (id 9463) in the full viewer
  • Open the measurement tool
  • Check that no message pops up. Check that you can see the roi

@jburel
Copy link
Member Author

jburel commented Jun 23, 2015

@will-moore: did you have a chance to look at it?

@will-moore
Copy link
Member

Stoke width looks same in Insight as web, but font size still doesn't.
Screenshot of web on left, insight on right.

screen shot 2015-06-24 at 00 54 10

@will-moore
Copy link
Member

NB: This is against local server (not rebuilt) so if this needs a server build then I'll test again (tomorrow!)

@mtbc
Copy link
Member

mtbc commented Jul 1, 2015

@jburel: Wondered if this needs relisting or fixing.

@jburel
Copy link
Member Author

jburel commented Jul 1, 2015

Fixing!!

@jburel
Copy link
Member Author

jburel commented Jul 3, 2015

@will-moore: how did you set the font size?
via the script?

@jburel
Copy link
Member Author

jburel commented Jul 3, 2015

Image from test 2 will show the issue.

@will-moore
Copy link
Member

@jburel Yes, with the roi.py script attached to the ticket above.

@jburel
Copy link
Member Author

jburel commented Jul 3, 2015

@will-moore: thanks found the source of the problem: conversion issue
I will fix it this evening

@jburel
Copy link
Member Author

jburel commented Jul 3, 2015

The font size should now be correctly handled
to test follow steps of Test 2

This is a difference between insight and web in the text display: location is not the same
This can be reviewed in another PR.

@gusferguson
Copy link

@jburel @will-moore

Test 1

  • Open the first image (id 9461) in the full viewer
  • Open the measurement tool
  • Check that the stroke width is displayed correctly i.e. red and width of 15 and 10

As expected

Test 2

  • Open the first image (id 9460) in the full viewer
  • Open the measurement tool
  • Check that no message pops up. Check that you can see the roi

ROIs did not display when Measurement Tool opened - had to select a ROI before they showed on the image.
No message popped up.

Test 3

  • Open the first image (id 9463) in the full viewer
  • Open the measurement tool
  • Check that no message pops up. Check that you can see the roi

ROIs showed on image when viewer opened before the measurement tool was opened.
No message popped up.

Comment from Will:

  • Stoke width looks same in Insight as web, but font size still doesn't.

None of the fonts are consistently displayed either within Insight or between Insight and Web.

  • 9461 - font same size in both but top aligned within ROI in Insight and centred in Web
  • 9460 - font outside ROI in Insight - presume because the size is too big to be within - ROIs not displayed at all in Web.
  • 9463 - font different size in Insight (smaller) and Web - also bottom aligned within ROI in Insight and centred in Web

See screenshot below.

Serious screwed behaviour here.
Is it because the ROIs were created by different methods?

main

@jburel
Copy link
Member Author

jburel commented Jul 6, 2015

@gusferguson:
@will-moore's script set the font size in pixels. By default we set in point. This is the main reason.
in this PR we no longer try to convert from pixels to point.
It will probably be preferable not to insert "invalid value" in the DB (i.e. font size not in pixels cc @joshmoore @mtbc)

As indicated in a previous comment, the location of the text is another problem.
This is not done in this PR.
The fact that it is not displayed in Web: case 2, @will-moore is aware of it and will fix it in another PR.

@will-moore
Copy link
Member

The reason that the ROIs aren't displayed in web or Insight when image ID 9460 is opened is because there are no shapes on the default Z / T plane. When you select a ROI/shape (as you did for Insight) then they are displayed (works for web too, see screenshot). So, it seems that that apart from text placement which is a known issue (and maybe choice of font-family), this PR is good to merge.

screen shot 2015-07-20 at 10 06 55

@@ -161,7 +161,9 @@ public Color getFill()
Shape shape = (Shape) asIObject();
RInt value = shape.getFillColor();
if (value == null) return DEFAULT_FILL_COLOUR;
return new Color(value.getValue(), true);
Color c = new Color(value.getValue(), true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB: seems like this might be a candidate for either a change to the existing Color constructor or a new one with another boolean flag lenient, etc.

joshmoore added a commit that referenced this pull request Jul 21, 2015
@joshmoore joshmoore merged commit 619b103 into ome:develop Jul 21, 2015
@joshmoore joshmoore added this to the 5.1.3 milestone Jul 24, 2015
@jburel jburel deleted the shape-font branch September 28, 2015 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants