Project

General

Profile

Bug #1708

charva/awt/ScreenBitmap.java cleanup

Added by Constantin Asofiei over 11 years ago. Updated over 11 years ago.

Status:
New
Priority:
Normal
Assignee:
-
Target version:
-
Start date:
Due date:
% Done:

0%

billable:
No
vendor_id:
GCD
case_num:
version:

History

#1 Updated by Constantin Asofiei over 11 years ago

Following are data about ScreenBitmap cleanup task. Attached are the mails between CA and NVS related to this cleanup. Main info is this:

NVS:
Since some revision of the ScreenBitMap is due anyway (remember those ynused extra positions in the map?), please mark this as a low priority work:

  • getting rid of extra positions of the bitmap (one extra position is created for every line)

NVS:
I've noticed that for my debugging session the screen bitmap size is 25x81 whereas the terminal window size is 24x80. Why does the ScreenBitmap implementation need those extras? I couldn't see they were used.
CA:
You are right, it is not used. I remember that I've added this protection at an early stage, which proved to be not-needed. If its not too much, you can remove it (just search for "new boolean"/Arrays.copyOf and remove the "+ 1"). It shouldn't affect anything.

  • reviewing the usefulness of hasRectangle, outerRectangle etc.

CA:
hasRectangle is a helper to determine if the screen was added any enabling rectangles. When it's FALSE, it indicates no area was explicitly enabled and so, no drawing is needed. This handles the null-rectangle case this way: if the screen bitmap is added a null rectangle (addRectangle receives a null param, entire screen is available for drawing), then hasRectangle is changed to TRUE, to allow drawing in Toolkit.setInvalidate(boolean, boolean).
NVS:
In general, the concept of outerRectangle is inverted in the implementation. All positions disabled truly means the outerRectangle has zero area, whereas all positions enabled should mean the outerRectangle matches the screen.
CA:
At that time, I chose outerRectangle == null test to determine if screen is enabled (instead of checking if outerRectangle matches the screen) as I thought this would be faster as comparing rectangle coordinates.

  • defining fully enabled and fully disabled state in a clean manner

    Problem with these flags is that they were introduced as an optimization. Currently, their state in certain cases is counter-intuitive (i.e. null outerRectangle means drawing is possible). After code review and cleaned, these flags may lose their purpose and may be removed.

  • documenting the design and the meaning of all state variables and operations, using the correct terminology, so that it's self-documented and comprehensive enough for anybody to understand the idea and implementation without going through all our emails

As some notions were confused in the ScreenBitmap javadoc's (i.e. clipping instead of enabling rectangles), it is necessary to review the fields and methods and check how they act, and if they behave in a non-confusing way. Also, please note even if clipping is mentioned in javadoc's, the author had in mind enabling rectangles.

  • documenting limitations, if any, and the reasons for their existence

Also available in: Atom PDF