apsexton / bateman-ocr

Tools and experiments in the OCR of the Bateman Manuscripts
ISC License
0 stars 5 forks source link

Add a toggle action to toggle display of the loaded image #10

Open apsexton opened 8 years ago

apsexton commented 8 years ago

Add a new action that toggles on or off the display of the loaded image.

This action should only be enabled when an image is loaded.

The action should have a suitable accelerator associated with it and be added to the menu and the toolbar

LibriCerule commented 8 years ago

How do you feel about me adding a field variable inside of Canvas that tracks the status of the toggle that decides whether to draw anything on the screen in the paintComponent method? I plan to change this every time the newly created button is clicked.

There is another problem with the implementation of enabling or disabling the toggle button. I was thinking of making the toggle button a field variable in View and create a get method that would allow the Model class to enable it in the loadImage method, but I think there may be a better way to do this. Do you have any suggestions for me?

apsexton commented 8 years ago

Such a field variable is appropriate. However, it shouldn't be in Canvas, though it is used there: this field is really about the view, not about the model, so it should go in the View class and Canvas should call it through its view field like it calls getImage() from the model field. In general, data that is information about the objects represented should be in Model, data about how to display those objects should be in View.

Once you have an Action object to trigger the toggle (just like openAction or exitAction), then such an object will have methods isEnabled() and setEnabled(...) which do all you need. All you need to do is enable the action when you load an image and disable it when (if) you close the image. The corresponding buttons/menuitems/keyboard accelerators will then be greyed out and impossible to trigger when the action is disabled. In general, work with actions as the main triggers for operations. Those actions can be attached to buttons, menu items and accelerators, but once they are attached, don't interact with those components directly, just manipulate the actions themselves.

LibriCerule commented 8 years ago

I enabled the toggleAction button inside of the adaptToNewImage method in the View class. I have a few questions about the button though. How would the user be able to close an image without actually closing the whole application? Also, when I toggle the display, the image disappears but there are still two scroll bars as if an image was still there. I simply do not render the actual image, meaning the user can still draw on the image but will not see any changes until he toggles again. Is there another way that I should implement toggling the display of the loaded image? Is there an image that I could use for this button and the next button for the next issue? I am currently using a placeholder for the images. I put in an intuitive description, but I don't know what shortcut to make it. Do you have any suggestions?

apsexton commented 8 years ago

Closing the image is not strictly necessary, but you can add that action for completeness if you want.

The scroll bars should stay because the image is (virtually) still there - just invisible - therefore the canvas is still stretched to contain it. This will make more sense when you implement the next issue.

For button icons, copy an existing icon and then edit it using the gimp or another picture editor. If using the gimp, you need to zoom in very far and set the pencil to a single pixel. You can also search for public domain icons and use them if you find something suitable.

Shortcuts are up to you - try to come up with something mnemonic, e.g. control-o for open etc.

apsexton commented 8 years ago

I have now been able to switch to the toggle branch and run the uploaded code. I think this is still an old version: your last commit recorded seems to be 9 days ago. You may have corrected this on your current version but when you toggle off the display of the loaded image, it should NOT disable the display of the bounding boxes, and it should fill the background with white: the idea is that our layout code will be depending ONLY on the bounding box information, not the character classifications, so we want to be able to see what the layout algorithm can see WITHOUT seeing what the characters actually are.

apsexton commented 8 years ago

Sorry - I closed this in error accidentally - just reopened to correct that